SERVER-33330 closeAll now checks for bg jobs in progress

Previously, closeAll() would have a "force" mode that caused it to either ignore bg jobs
in progress (force on), or avoid closing databases with bg jobs in progress.  This parameter
is no longer useful, so I have removed it.  The test-only restartCatalog command will now
return an error instead of crashing if you call it with bg jobs in progress.

7 files changed, 14 insertions, 67 deletions

diff --git a/src/mongo/db/catalog/catalog_control.cpp b/src/mongo/db/catalog/catalog_control.cpp
index bf4d6db2653..fad8950aed0 100644
--- a/src/mongo/db/catalog/catalog_control.cpp
+++ b/src/mongo/db/catalog/catalog_control.cpp
@@ -47,21 +47,9 @@ void closeCatalog(OperationContext* opCtx) {
     invariant(opCtx->lockState()->isW());
 
     // Close all databases.
-    log() << "closeCatalog: closing all databases in dbholder";
-    BSONObjBuilder closeDbsBuilder;
-    constexpr auto force = true;
+    log() << "closeCatalog: closing all databases";
     constexpr auto reason = "closing databases for closeCatalog";
-    uassert(40687,
-            str::stream() << "failed to close all databases; result of operation: "
-                          << closeDbsBuilder.obj().jsonString(),
-            dbHolder().closeAll(opCtx, closeDbsBuilder, force, reason));
-
-    // Because we've force-closed the database, there should be no databases left open.
-    auto closeDbsResult = closeDbsBuilder.obj();
-    invariant(
-        !closeDbsResult.hasField("nNotClosed"),
-        str::stream() << "expected no databases open after a force close; result of operation: "
-                      << closeDbsResult.jsonString());
+    dbHolder().closeAll(opCtx, reason);
 
     // Close the storage engine's catalog.
     log() << "closeCatalog: closing storage engine catalog";
diff --git a/src/mongo/db/catalog/database_holder.h b/src/mongo/db/catalog/database_holder.h
index 58af1747218..0edde9cfccf 100644
--- a/src/mongo/db/catalog/database_holder.h
+++ b/src/mongo/db/catalog/database_holder.h
@@ -57,10 +57,7 @@ public:
 
         virtual void close(OperationContext* opCtx, StringData ns, const std::string& reason) = 0;
 
-        virtual bool closeAll(OperationContext* opCtx,
-                              BSONObjBuilder& result,
-                              bool force,
-                              const std::string& reason) = 0;
+        virtual void closeAll(OperationContext* opCtx, const std::string& reason) = 0;
 
         virtual std::set<std::string> getNamesWithConflictingCasing(StringData name) = 0;
     };
@@ -100,6 +97,7 @@ public:
 
     /**
      * Closes the specified database. Must be called with the database locked in X-mode.
+     * No background jobs must be in progress on the database when this function is called.
      */
     inline void close(OperationContext* const opCtx,
                       const StringData ns,
@@ -109,16 +107,12 @@ public:
 
     /**
      * Closes all opened databases. Must be called with the global lock acquired in X-mode.
+     * Will uassert if any background jobs are running when this function is called.
      *
-     * @param result Populated with the names of the databases, which were closed.
-     * @param force Force close even if something underway - use at shutdown
      * @param reason The reason for close.
      */
-    inline bool closeAll(OperationContext* const opCtx,
-                         BSONObjBuilder& result,
-                         const bool force,
-                         const std::string& reason) {
-        return this->_impl().closeAll(opCtx, result, force, reason);
+    inline void closeAll(OperationContext* const opCtx, const std::string& reason) {
+        this->_impl().closeAll(opCtx, reason);
     }
 
     /**
diff --git a/src/mongo/db/catalog/database_holder_impl.cpp b/src/mongo/db/catalog/database_holder_impl.cpp
index 62f424e8b2a..1c999013e33 100644
--- a/src/mongo/db/catalog/database_holder_impl.cpp
+++ b/src/mongo/db/catalog/database_holder_impl.cpp
@@ -221,10 +221,7 @@ void DatabaseHolderImpl::close(OperationContext* opCtx, StringData ns, const std
         .transitional_ignore();
 }
 
-bool DatabaseHolderImpl::closeAll(OperationContext* opCtx,
-                                  BSONObjBuilder& result,
-                                  bool force,
-                                  const std::string& reason) {
+void DatabaseHolderImpl::closeAll(OperationContext* opCtx, const std::string& reason) {
     invariant(opCtx->lockState()->isW());
 
     stdx::lock_guard<SimpleMutex> lk(_m);
@@ -234,19 +231,12 @@ bool DatabaseHolderImpl::closeAll(OperationContext* opCtx,
         dbs.insert(i->first);
     }
 
-    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;
 
-        if (!force && BackgroundOperation::inProgForDb(name)) {
-            log() << "WARNING: can't close database " << name
-                  << " because a bg job is in progress - try killOp command";
-            nNotClosed++;
-            continue;
-        }
+        BackgroundOperation::assertNoBgOpInProgForDb(name);
 
         Database* db = _dbs[name];
         repl::oplogCheckCloseDatabase(opCtx, db);
@@ -260,15 +250,6 @@ bool DatabaseHolderImpl::closeAll(OperationContext* opCtx,
             ->getGlobalStorageEngine()
             ->closeDatabase(opCtx, name)
             .transitional_ignore();
-
-        bb.append(name);
-    }
-
-    bb.done();
-    if (nNotClosed) {
-        result.append("nNotClosed", nNotClosed);
     }
-
-    return true;
 }
 }  // namespace mongo
diff --git a/src/mongo/db/catalog/database_holder_impl.h b/src/mongo/db/catalog/database_holder_impl.h
index c60d0043123..4cf4627857c 100644
--- a/src/mongo/db/catalog/database_holder_impl.h
+++ b/src/mongo/db/catalog/database_holder_impl.h
@@ -68,20 +68,17 @@ public:
 
     /**
      * Closes the specified database. Must be called with the database locked in X-mode.
+     * No background jobs must be in progress on the database when this function is called.
      */
     void close(OperationContext* opCtx, StringData ns, const std::string& reason) override;
 
     /**
      * Closes all opened databases. Must be called with the global lock acquired in X-mode.
+     * Will uassert if any background jobs are running when this function is called.
      *
-     * @param result Populated with the names of the databases, which were closed.
-     * @param force Force close even if something underway - use at shutdown
      * @param reason The reason for close.
      */
-    bool closeAll(OperationContext* opCtx,
-                  BSONObjBuilder& result,
-                  bool force,
-                  const std::string& reason) override;
+    void closeAll(OperationContext* opCtx, const std::string& reason) override;
 
     /**
      * Returns the set of existing database names that differ only in casing.
diff --git a/src/mongo/db/catalog/database_holder_mock.h b/src/mongo/db/catalog/database_holder_mock.h
index 73172081f6a..ab4058e4417 100644
--- a/src/mongo/db/catalog/database_holder_mock.h
+++ b/src/mongo/db/catalog/database_holder_mock.h
@@ -74,16 +74,9 @@ public:
     /**
      * Closes all opened databases. Must be called with the global lock acquired in X-mode.
      *
-     * result Populated with the names of the databases, which were closed.
-     * force Force close even if something underway - use at shutdown
      * reason The reason for close.
      */
-    bool closeAll(OperationContext* opCtx,
-                  BSONObjBuilder& result,
-                  bool force,
-                  const std::string& reason) override {
-        return true;
-    }
+    void closeAll(OperationContext* opCtx, const std::string& reason) override {}
 
     /**
      * Returns the set of existing database names that differ only in casing.
diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp
index b98dcc08390..d81fcef249e 100644
--- a/src/mongo/db/catalog/database_impl.cpp
+++ b/src/mongo/db/catalog/database_impl.cpp
@@ -161,16 +161,11 @@ DatabaseImpl::~DatabaseImpl() {
 }
 
 void DatabaseImpl::close(OperationContext* opCtx, const std::string& reason) {
-    // XXX? - Do we need to close database under global lock or just DB-lock is sufficient ?
     invariant(opCtx->lockState()->isW());
 
     // Clear cache of oplog Collection pointer.
     repl::oplogCheckCloseDatabase(opCtx, this->_this);
 
-    if (BackgroundOperation::inProgForDb(_name)) {
-        log() << "warning: bg op in prog during close db? " << _name;
-    }
-
     for (auto&& pair : _collections) {
         auto* coll = pair.second;
         coll->getCursorManager()->invalidateAll(opCtx, true, reason);
diff --git a/src/mongo/db/service_context_d_test_fixture.cpp b/src/mongo/db/service_context_d_test_fixture.cpp
index ab8939ed51f..cc1401b6517 100644
--- a/src/mongo/db/service_context_d_test_fixture.cpp
+++ b/src/mongo/db/service_context_d_test_fixture.cpp
@@ -114,8 +114,7 @@ void ServiceContextMongoDTest::_dropAllDBs(OperationContext* opCtx) {
     // dropAllDatabasesExceptLocal() does not close empty databases. However the holder still
     // allocates resources to track these empty databases. These resources not released by
     // dropAllDatabasesExceptLocal() will be leaked at exit unless we call DatabaseHolder::closeAll.
-    BSONObjBuilder unused;
-    invariant(dbHolder().closeAll(opCtx, unused, false, "all databases dropped"));
+    dbHolder().closeAll(opCtx, "all databases dropped");
 }
 
 }  // namespace mongo