SERVER-13635: clean database close path

4 files changed, 48 insertions, 20 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,