SERVER-16413 Storage engines that allow online compaction should not take an exclusive lock

5 files changed, 74 insertions, 37 deletions

diff --git a/src/mongo/db/catalog/collection_compact.cpp b/src/mongo/db/catalog/collection_compact.cpp
index c3e886d27eb..60c06b561e0 100644
--- a/src/mongo/db/catalog/collection_compact.cpp
+++ b/src/mongo/db/catalog/collection_compact.cpp
@@ -31,22 +31,58 @@
 
 #include "mongo/db/catalog/collection_compact.h"
 
+#include "mongo/db/catalog/collection_catalog.h"
 #include "mongo/db/catalog/document_validation.h"
 #include "mongo/db/catalog/index_key_validate.h"
 #include "mongo/db/catalog/multi_index_block.h"
+#include "mongo/db/db_raii.h"
 #include "mongo/db/index/index_access_method.h"
 #include "mongo/db/index/index_descriptor.h"
+#include "mongo/db/index_builds_coordinator.h"
 #include "mongo/db/operation_context.h"
+#include "mongo/db/views/view_catalog.h"
 #include "mongo/util/assert_util.h"
 #include "mongo/util/log.h"
 
 namespace mongo {
 
+using logger::LogComponent;
+
+namespace {
+
+Collection* getCollectionForCompact(OperationContext* opCtx,
+                                    Database* database,
+                                    const NamespaceString& collectionNss) {
+    invariant(opCtx->lockState()->isCollectionLockedForMode(collectionNss, MODE_IX));
+
+    CollectionCatalog& collectionCatalog = CollectionCatalog::get(opCtx);
+    Collection* collection = collectionCatalog.lookupCollectionByNamespace(collectionNss);
+
+    if (!collection) {
+        std::shared_ptr<ViewDefinition> view =
+            ViewCatalog::get(database)->lookup(opCtx, collectionNss.ns());
+        uassert(ErrorCodes::CommandNotSupportedOnView, "can't compact a view", !view);
+        uasserted(ErrorCodes::NamespaceNotFound, "collection does not exist");
+    }
+
+    return collection;
+}
+
+}  // namespace
+
 StatusWith<CompactStats> compactCollection(OperationContext* opCtx,
-                                           Collection* collection,
+                                           const NamespaceString& collectionNss,
                                            const CompactOptions* compactOptions) {
-    dassert(opCtx->lockState()->isCollectionLockedForMode(collection->ns(), MODE_X));
+    AutoGetDb autoDb(opCtx, collectionNss.db(), MODE_IX);
+    Database* database = autoDb.getDb();
+    uassert(ErrorCodes::NamespaceNotFound, "database does not exist", database);
+
+    // The collection lock will be downgraded to an intent lock if the record store supports
+    // online compaction.
+    boost::optional<Lock::CollectionLock> collLk;
+    collLk.emplace(opCtx, collectionNss, MODE_X);
 
+    Collection* collection = getCollectionForCompact(opCtx, database, collectionNss);
     DisableDocumentValidation validationDisabler(opCtx);
 
     auto recordStore = collection->getRecordStore();
@@ -58,6 +94,19 @@ StatusWith<CompactStats> compactCollection(OperationContext* opCtx,
                                             << "cannot compact collection with record store: "
                                             << recordStore->name());
 
+    if (recordStore->supportsOnlineCompaction()) {
+        // Storage engines that allow online compaction should do so using an intent lock on the
+        // collection.
+        collLk.emplace(opCtx, collectionNss, MODE_IX);
+
+        // Ensure the collection was not dropped during the re-lock.
+        collection = getCollectionForCompact(opCtx, database, collectionNss);
+    }
+
+    OldClientContext ctx(opCtx, collectionNss.ns());
+    log(LogComponent::kCommand) << "compact " << collectionNss
+                                << " begin, options: " << *compactOptions;
+
     if (recordStore->compactsInPlace()) {
         CompactStats stats;
         Status status = recordStore->compact(opCtx);
@@ -69,12 +118,17 @@ StatusWith<CompactStats> compactCollection(OperationContext* opCtx,
         if (!status.isOK())
             return StatusWith<CompactStats>(status);
 
+        log() << "compact " << collectionNss << " end";
         return StatusWith<CompactStats>(stats);
     }
 
-    if (indexCatalog->numIndexesInProgress(opCtx))
-        return StatusWith<CompactStats>(ErrorCodes::BadValue,
-                                        "cannot compact when indexes in progress");
+    invariant(opCtx->lockState()->isCollectionLockedForMode(collectionNss, MODE_X));
+
+    // If the storage engine doesn't support compacting in place, make sure no background operations
+    // or indexes are running.
+    const UUID collectionUUID = collection->uuid().get();
+    BackgroundOperation::assertNoBgOpInProgForNs(collectionNss);
+    IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection(collectionUUID);
 
     std::vector<BSONObj> indexSpecs;
     {
@@ -148,6 +202,7 @@ StatusWith<CompactStats> compactCollection(OperationContext* opCtx,
         wunit.commit();
     }
 
+    log() << "compact " << collectionNss << " end";
     return StatusWith<CompactStats>(stats);
 }
 
diff --git a/src/mongo/db/catalog/collection_compact.h b/src/mongo/db/catalog/collection_compact.h
index b17b0ec3886..a3aefc1833e 100644
--- a/src/mongo/db/catalog/collection_compact.h
+++ b/src/mongo/db/catalog/collection_compact.h
@@ -30,7 +30,6 @@
 #pragma once
 
 #include "mongo/base/status_with.h"
-#include "mongo/db/catalog/collection.h"
 #include "mongo/db/storage/record_store.h"
 
 namespace mongo {
@@ -40,7 +39,7 @@ namespace mongo {
   * See record_store.h for CompactStats and CompactOptions definitions.
   */
 StatusWith<CompactStats> compactCollection(OperationContext* opCtx,
-                                           Collection* collection,
+                                           const NamespaceString& collectionNss,
                                            const CompactOptions* options);
 
 }  // namespace mongo
diff --git a/src/mongo/db/commands/compact.cpp b/src/mongo/db/commands/compact.cpp
index d78af120927..a1262f66412 100644
--- a/src/mongo/db/commands/compact.cpp
+++ b/src/mongo/db/commands/compact.cpp
@@ -42,8 +42,6 @@
 #include "mongo/db/commands.h"
 #include "mongo/db/concurrency/d_concurrency.h"
 #include "mongo/db/curop.h"
-#include "mongo/db/db_raii.h"
-#include "mongo/db/index_builds_coordinator.h"
 #include "mongo/db/jsobj.h"
 #include "mongo/db/repl/replication_coordinator.h"
 #include "mongo/db/views/view_catalog.h"
@@ -117,34 +115,7 @@ public:
         if (cmdObj.hasElement("validate"))
             compactOptions.validateDocuments = cmdObj["validate"].trueValue();
 
-        AutoGetDb autoDb(opCtx, db, MODE_X);
-        Database* const collDB = autoDb.getDb();
-
-        Collection* collection = collDB ? collDB->getCollection(opCtx, nss) : nullptr;
-        auto view =
-            collDB && !collection ? ViewCatalog::get(collDB)->lookup(opCtx, nss.ns()) : nullptr;
-
-        // If db/collection does not exist, short circuit and return.
-        if (!collDB || !collection) {
-            if (view)
-                uasserted(ErrorCodes::CommandNotSupportedOnView, "can't compact a view");
-            else
-                uasserted(ErrorCodes::NamespaceNotFound, "collection does not exist");
-        }
-
-        OldClientContext ctx(opCtx, nss.ns());
-        BackgroundOperation::assertNoBgOpInProgForNs(nss.ns());
-        invariant(collection->uuid());
-        IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection(
-            collection->uuid().get());
-
-        log() << "compact " << nss.ns() << " begin, options: " << compactOptions;
-
-        StatusWith<CompactStats> status = compactCollection(opCtx, collection, &compactOptions);
-        uassertStatusOK(status.getStatus());
-
-        log() << "compact " << nss.ns() << " end";
-
+        uassertStatusOK(compactCollection(opCtx, nss, &compactOptions));
         return true;
     }
 };
diff --git a/src/mongo/db/storage/record_store.h b/src/mongo/db/storage/record_store.h
index 93f01d8c81a..d4a5151faff 100644
--- a/src/mongo/db/storage/record_store.h
+++ b/src/mongo/db/storage/record_store.h
@@ -488,6 +488,15 @@ public:
     }
 
     /**
+     * If compact() supports online compaction.
+     *
+     * Only called if compactSupported() returns true.
+     */
+    virtual bool supportsOnlineCompaction() const {
+        MONGO_UNREACHABLE;
+    }
+
+    /**
      * Attempt to reduce the storage space used by this RecordStore.
      *
      * Only called if compactSupported() returns true.
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
index 8bdfdaecdd6..b448e76ea52 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
@@ -178,6 +178,9 @@ public:
     virtual bool compactsInPlace() const {
         return true;
     }
+    virtual bool supportsOnlineCompaction() const {
+        return true;
+    }
 
     virtual Timestamp getPinnedOplog() const final;