diff options
author | Eliot Horowitz <eliot@10gen.com> | 2014-08-04 09:11:12 -0400 |
---|---|---|
committer | Eliot Horowitz <eliot@10gen.com> | 2014-08-05 08:51:41 -0400 |
commit | f8003b264fe6c7a73fdfefd969983a885ef52958 (patch) | |
tree | ede5a8393baa1d5b238c94e55e48d525ae2eb04a /src | |
parent | 931f48190b276dd783d76ae4071228d6ab1f0bfd (diff) | |
download | mongo-f8003b264fe6c7a73fdfefd969983a885ef52958.tar.gz |
SERVER-13635: clean database close path
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/catalog/database.cpp | 18 | ||||
-rw-r--r-- | src/mongo/db/catalog/database.h | 13 | ||||
-rw-r--r-- | src/mongo/db/catalog/database_holder.cpp | 34 | ||||
-rw-r--r-- | src/mongo/db/catalog/database_holder.h | 3 | ||||
-rw-r--r-- | src/mongo/db/db.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/instance.cpp | 24 | ||||
-rw-r--r-- | src/mongo/db/storage/mmap_v1/repair_database.cpp | 3 |
7 files changed, 51 insertions, 46 deletions
diff --git a/src/mongo/db/catalog/database.cpp b/src/mongo/db/catalog/database.cpp index b9c1b82e59b..f3c8179b9e7 100644 --- a/src/mongo/db/catalog/database.cpp +++ b/src/mongo/db/catalog/database.cpp @@ -85,6 +85,22 @@ namespace mongo { delete i->second; } + void Database::close(OperationContext* txn ) { + // XXX? - Do we need to close database under global lock or just DB-lock is sufficient ? + invariant(txn->lockState()->isW()); + + repl::oplogCheckCloseDatabase(txn, this); // oplog caches some things, dirty its caches + + if ( BackgroundOperation::inProgForDb( _name ) ) { + log() << "warning: bg op in prog during close db? " << _name << endl; + } + + // Before the files are closed, flush any potentially outstanding changes, which might + // reference this database. Otherwise we will assert when subsequent commit if needed + // is called and it happens to have write intents for the removed files. + txn->recoveryUnit()->commitIfNeeded(true); + } + Status Database::validateDBName( const StringData& dbname ) { if ( dbname.size() <= 0 ) @@ -552,7 +568,7 @@ namespace mongo { txn->recoveryUnit()->syncDataAndTruncateJournal(); - Database::closeDatabase(txn, name ); + dbHolder().close( txn, name ); db = 0; // d is now deleted _deleteDataFiles( name ); diff --git a/src/mongo/db/catalog/database.h b/src/mongo/db/catalog/database.h index 4d8ebf3b128..2b080f063de 100644 --- a/src/mongo/db/catalog/database.h +++ b/src/mongo/db/catalog/database.h @@ -61,14 +61,11 @@ namespace mongo { const StringData& name, DatabaseCatalogEntry* dbEntry ); - /* you must use this to close - there is essential code in this method that is not in the ~Database destructor. - thus the destructor is private. this could be cleaned up one day... - */ - static void closeDatabase(OperationContext* txn, - const StringData& db); - - // do not use! - ~Database(); // closes files and other cleanup see below. + // must call close first + ~Database(); + + // closes files and other cleanup see below. + void close( OperationContext* txn ); const std::string& name() const { return _name; } diff --git a/src/mongo/db/catalog/database_holder.cpp b/src/mongo/db/catalog/database_holder.cpp index 22c1b300537..8b494631d67 100644 --- a/src/mongo/db/catalog/database_holder.cpp +++ b/src/mongo/db/catalog/database_holder.cpp @@ -121,12 +121,20 @@ namespace mongo { return db; } - void DatabaseHolder::erase(OperationContext* txn, + void DatabaseHolder::close(OperationContext* txn, const StringData& ns) { invariant(txn->lockState()->isW()); + StringData db = _todb(ns); + SimpleMutex::scoped_lock lk(_m); - _dbs.erase(_todb(ns)); + DBs::const_iterator it = _dbs.find(db); + if ( it == _dbs.end() ) + return; + + it->second->close( txn ); + delete it->second; + _dbs.erase( db ); } bool DatabaseHolder::closeAll(OperationContext* txn, @@ -136,32 +144,38 @@ namespace mongo { getDur().commitNow(txn); // bad things happen if we close a DB with outstanding writes + SimpleMutex::scoped_lock lk(_m); + set< string > dbs; for ( DBs::const_iterator i = _dbs.begin(); i != _dbs.end(); ++i ) { dbs.insert( i->first ); } - BSONObjBuilder bb( result.subarrayStart( "dbs" ) ); - int n = 0; + BSONArrayBuilder bb( result.subarrayStart( "dbs" ) ); int nNotClosed = 0; for( set< string >::iterator i = dbs.begin(); i != dbs.end(); ++i ) { string name = *i; LOG(2) << "DatabaseHolder::closeAll name:" << name; - Client::Context ctx(txn, name); if( !force && BackgroundOperation::inProgForDb(name) ) { log() << "WARNING: can't close database " << name - << " because a bg job is in progress - try killOp command" + << " because a bg job is in progress - try killOp command" << endl; nNotClosed++; + continue; } - else { - Database::closeDatabase(txn, name.c_str()); - bb.append( bb.numStr( n++ ) , name ); - } + + Database* db = _dbs[name]; + db->close( txn ); + delete db; + + _dbs.erase( name ); + + bb.append( name ); } + bb.done(); if( nNotClosed ) { result.append("nNotClosed", nNotClosed); diff --git a/src/mongo/db/catalog/database_holder.h b/src/mongo/db/catalog/database_holder.h index b6c874bf7a3..bd3b3cd0133 100644 --- a/src/mongo/db/catalog/database_holder.h +++ b/src/mongo/db/catalog/database_holder.h @@ -56,7 +56,8 @@ namespace mongo { const StringData& ns, bool& justCreated); - void erase(OperationContext* txn, const StringData& ns); + + void close(OperationContext* txn, const StringData& ns); /** @param force - force close even if something underway - use at shutdown */ bool closeAll(OperationContext* txn, diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp index 4ccc49f3d26..4b63572902a 100644 --- a/src/mongo/db/db.cpp +++ b/src/mongo/db/db.cpp @@ -409,7 +409,7 @@ namespace mongo { warning() << "Internal error while reading collection " << systemIndexes; } - Database::closeDatabase(&txn, dbName.c_str()); + dbHolder().close( &txn, dbName ); } } wunit.commit(); diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp index e9b2fe966fb..025da941a20 100644 --- a/src/mongo/db/instance.cpp +++ b/src/mongo/db/instance.cpp @@ -545,30 +545,6 @@ namespace mongo { } - /*static*/ - void Database::closeDatabase(OperationContext* txn, const StringData& db) { - // XXX? - Do we need to close database under global lock or just DB-lock is sufficient ? - invariant(txn->lockState()->isW()); - - Database* database = dbHolder().get(txn, db); - if ( !database ) - return; - - repl::oplogCheckCloseDatabase(txn, database); // oplog caches some things, dirty its caches - - if( BackgroundOperation::inProgForDb(db) ) { - log() << "warning: bg op in prog during close db? " << db << endl; - } - - // Before the files are closed, flush any potentially outstanding changes, which might - // reference this database. Otherwise we will assert when subsequent commit if needed - // is called and it happens to have write intents for the removed files. - txn->recoveryUnit()->commitIfNeeded(true); - - dbHolder().erase(txn, db); - delete database; // closes files - } - void receivedUpdate(OperationContext* txn, Message& m, CurOp& op) { DbMessage d(m); NamespaceString ns(d.getns()); diff --git a/src/mongo/db/storage/mmap_v1/repair_database.cpp b/src/mongo/db/storage/mmap_v1/repair_database.cpp index 8fb06a5eae6..9a6775e7e9b 100644 --- a/src/mongo/db/storage/mmap_v1/repair_database.cpp +++ b/src/mongo/db/storage/mmap_v1/repair_database.cpp @@ -451,8 +451,9 @@ namespace mongo { if ( repairFileDeleter.get() ) repairFileDeleter->success(); + dbHolder().close( txn, dbName ); + Client::Context ctx(txn, dbName); - Database::closeDatabase(txn, dbName); if ( backupOriginalFiles ) { _renameForBackup( dbName, reservedPath ); |