SERVER-65261 User deletes on capped collections do not return CappedPositionLost

15 files changed, 106 insertions, 22 deletions

diff --git a/jstests/replsets/capped_deletes.js b/jstests/replsets/capped_deletes.js
new file mode 100644
index 00000000000..78b55691576
--- /dev/null
+++ b/jstests/replsets/capped_deletes.js
@@ -0,0 +1,49 @@
+/**
+ * Tests user deletes on capped collections.
+ *
+ * @tags: [multiversion_incompatible]
+ */
+(function() {
+"use strict";
+
+let replTest = new ReplSetTest({name: "capped_deletes", nodes: 2});
+replTest.startSet();
+replTest.initiate();
+
+let primary = replTest.getPrimary();
+
+let dbName = "test";
+let collName = "capped_deletes";
+
+let db = primary.getDB(dbName);
+
+assert.commandWorked(db.createCollection(collName, {capped: true, size: 4096}));
+
+let coll = db.getCollection(collName);
+
+for (let i = 0; i < 10; i++) {
+    coll.insert({_id: i});
+}
+
+let res = coll.runCommand("delete", {deletes: [{q: {_id: 2}, limit: 1}]});
+assert.eq(coll.find().itcount(), 9, res);
+assert.eq(res.n, 1, res);
+assert.commandWorked(res);
+
+res = coll.runCommand("delete", {deletes: [{q: {}, limit: 1, hint: {_id: 1}}]});
+assert.eq(coll.find().itcount(), 8, res);
+assert.eq(res.n, 1, res);
+assert.commandWorked(res);
+
+res = coll.runCommand("delete", {deletes: [{q: {_id: 5}, limit: 1, hint: {$natural: 1}}]});
+assert.eq(coll.find().itcount(), 7, res);
+assert.eq(res.n, 1, res);
+assert.commandWorked(res);
+
+res = coll.runCommand("delete", {deletes: [{q: {}, limit: 0}]});
+assert.eq(coll.find().itcount(), 0, res);
+assert.eq(res.n, 7, res);
+assert.commandWorked(res);
+
+replTest.stopSet();
+}());
diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp
index ba41678fd2b..4e15e370a20 100644
--- a/src/mongo/db/catalog/collection_impl.cpp
+++ b/src/mongo/db/catalog/collection_impl.cpp
@@ -1157,6 +1157,15 @@ void CollectionImpl::deleteDocument(OperationContext* opCtx,
         }
     }
 
+    if (_shared->_needCappedLock) {
+        // X-lock the metadata resource for this capped collection until the end of the WUOW. This
+        // prevents the primary from executing with more concurrency than secondaries and protects
+        // '_cappedFirstRecord'.
+        // See SERVER-21646.
+        Lock::ResourceLock heldUntilEndOfWUOW{
+            opCtx->lockState(), ResourceId(RESOURCE_METADATA, _ns.ns()), MODE_X};
+    }
+
     std::vector<OplogSlot> oplogSlots;
     if (storeDeletedDoc == Collection::StoreDeletedDoc::On && !getRecordPreImages()) {
         oplogSlots = reserveOplogSlotsForRetryableFindAndModify(opCtx, 2);
diff --git a/src/mongo/db/exec/collection_scan.cpp b/src/mongo/db/exec/collection_scan.cpp
index 7f3d5bc446c..c601e4d773c 100644
--- a/src/mongo/db/exec/collection_scan.cpp
+++ b/src/mongo/db/exec/collection_scan.cpp
@@ -299,7 +299,8 @@ void CollectionScan::doSaveStateRequiresCollection() {
 
 void CollectionScan::doRestoreStateRequiresCollection() {
     if (_cursor) {
-        const bool couldRestore = _cursor->restore();
+        const auto tolerateCappedCursorRepositioning = expCtx()->getIsCappedDelete();
+        const bool couldRestore = _cursor->restore(tolerateCappedCursorRepositioning);
         uassert(ErrorCodes::CappedPositionLost,
                 str::stream()
                     << "CollectionScan died due to position in capped collection being deleted. "
diff --git a/src/mongo/db/exec/sbe/stages/scan.cpp b/src/mongo/db/exec/sbe/stages/scan.cpp
index 3be69da9c92..cc69e0905c6 100644
--- a/src/mongo/db/exec/sbe/stages/scan.cpp
+++ b/src/mongo/db/exec/sbe/stages/scan.cpp
@@ -192,7 +192,8 @@ void ScanStage::doRestoreState() {
     _coll = restoreCollection(_opCtx, _collName, _collUuid, _catalogEpoch);
 
     if (_cursor) {
-        const bool couldRestore = _cursor->restore();
+        const auto tolerateCappedCursorRepositioning = false;
+        const bool couldRestore = _cursor->restore(tolerateCappedCursorRepositioning);
         uassert(ErrorCodes::CappedPositionLost,
                 str::stream()
                     << "CollectionScan died due to position in capped collection being deleted. ",
diff --git a/src/mongo/db/pipeline/document_source_check_resume_token_test.cpp b/src/mongo/db/pipeline/document_source_check_resume_token_test.cpp
index e8fde0f3d7c..bd0f59b0b29 100644
--- a/src/mongo/db/pipeline/document_source_check_resume_token_test.cpp
+++ b/src/mongo/db/pipeline/document_source_check_resume_token_test.cpp
@@ -82,7 +82,7 @@ public:
         return boost::none;
     }
     void save() override {}
-    bool restore() override {
+    bool restore(bool tolerateCappedRepositioning) override {
         return true;
     }
     void detachFromOperationContext() override {}
diff --git a/src/mongo/db/pipeline/expression_context.h b/src/mongo/db/pipeline/expression_context.h
index 0b54b98f898..c1dae29660a 100644
--- a/src/mongo/db/pipeline/expression_context.h
+++ b/src/mongo/db/pipeline/expression_context.h
@@ -289,6 +289,14 @@ public:
         _resolvedNamespaces = std::move(resolvedNamespaces);
     }
 
+    void setIsCappedDelete() {
+        _isCappedDelete = true;
+    }
+
+    bool getIsCappedDelete() const {
+        return _isCappedDelete;
+    }
+
     /**
      * Retrieves the Javascript Scope for the current thread or creates a new one if it has not been
      * created yet. Initializes the Scope with the 'jsScope' variables from the runtimeConstants.
@@ -455,6 +463,8 @@ protected:
 
     int _interruptCounter = kInterruptCheckPeriod;
 
+    bool _isCappedDelete = false;
+
 private:
     boost::optional<ExpressionCounters> _expressionCounters = boost::none;
 };
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index 7acf1ee351d..28e3c96934b 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -1328,6 +1328,10 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDele
     }
 
     if (collection && collection->isCapped()) {
+        expCtx->setIsCappedDelete();
+    }
+
+    if (collection && collection->isCapped()) {
         // This check is duplicated from CollectionImpl::deleteDocument() for two reasons:
         // - Performing a remove on an empty capped collection would not call
         //   CollectionImpl::deleteDocument().
diff --git a/src/mongo/db/storage/devnull/devnull_kv_engine.cpp b/src/mongo/db/storage/devnull/devnull_kv_engine.cpp
index 0855f2842d0..413dee06859 100644
--- a/src/mongo/db/storage/devnull/devnull_kv_engine.cpp
+++ b/src/mongo/db/storage/devnull/devnull_kv_engine.cpp
@@ -51,7 +51,7 @@ public:
         return {};
     }
     void save() final {}
-    bool restore() final {
+    bool restore(bool tolerateCappedRepositioning = true) final {
         return true;
     }
     void detachFromOperationContext() final {}
diff --git a/src/mongo/db/storage/devnull/ephemeral_catalog_record_store.cpp b/src/mongo/db/storage/devnull/ephemeral_catalog_record_store.cpp
index a9a1f7f2776..7beff2041dc 100644
--- a/src/mongo/db/storage/devnull/ephemeral_catalog_record_store.cpp
+++ b/src/mongo/db/storage/devnull/ephemeral_catalog_record_store.cpp
@@ -163,7 +163,7 @@ public:
         _savedId = RecordId();
     }
 
-    bool restore() final {
+    bool restore(bool tolerateCappedRepositioning) final {
         if (_savedId.isNull()) {
             _it = _records.end();
             return true;
@@ -241,7 +241,7 @@ public:
         _savedId = RecordId();
     }
 
-    bool restore() final {
+    bool restore(bool tolerateCappedRepositioning) final {
         if (_savedId.isNull()) {
             _it = _records.rend();
             return true;
diff --git a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_kv_engine.cpp b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_kv_engine.cpp
index f51e580fdc6..4feabb7374f 100644
--- a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_kv_engine.cpp
+++ b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_kv_engine.cpp
@@ -272,7 +272,7 @@ public:
         return {};
     }
     void save() final {}
-    bool restore() final {
+    bool restore(bool tolerateCappedRepositioning) final {
         return true;
     }
     void detachFromOperationContext() final {}
diff --git a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.cpp b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.cpp
index dc2ba7b4e3b..a0e6101b3a6 100644
--- a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.cpp
+++ b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.cpp
@@ -461,7 +461,7 @@ void RecordStore::Cursor::saveUnpositioned() {
     _savedPosition = boost::none;
 }
 
-bool RecordStore::Cursor::restore() {
+bool RecordStore::Cursor::restore(bool tolerateCappedRepositioning) {
     if (!_savedPosition)
         return true;
 
@@ -588,7 +588,7 @@ void RecordStore::ReverseCursor::saveUnpositioned() {
     _savedPosition = boost::none;
 }
 
-bool RecordStore::ReverseCursor::restore() {
+bool RecordStore::ReverseCursor::restore(bool tolerateCappedRepositioning) {
     if (!_savedPosition)
         return true;
 
diff --git a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.h b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.h
index 02dd90fe4da..26bd735920c 100644
--- a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.h
+++ b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.h
@@ -181,7 +181,7 @@ private:
         boost::optional<Record> seekNear(const RecordId& id) final override;
         void save() final;
         void saveUnpositioned() final override;
-        bool restore() final;
+        bool restore(bool tolerateCappedRepositioning = true) final;
         void detachFromOperationContext() final;
         void reattachToOperationContext(OperationContext* opCtx) final;
 
@@ -207,7 +207,7 @@ private:
         boost::optional<Record> seekNear(const RecordId& id) final override;
         void save() final;
         void saveUnpositioned() final override;
-        bool restore() final;
+        bool restore(bool tolerateCappedRepositioning = true) final;
         void detachFromOperationContext() final;
         void reattachToOperationContext(OperationContext* opCtx) final;
 
diff --git a/src/mongo/db/storage/record_store.h b/src/mongo/db/storage/record_store.h
index fc279494b82..19ed5101c41 100644
--- a/src/mongo/db/storage/record_store.h
+++ b/src/mongo/db/storage/record_store.h
@@ -132,15 +132,18 @@ public:
      * Returns false if it is invalid to continue using this Cursor. This usually means that
      * capped deletes have caught up to the position of this Cursor and continuing could
      * result in missed data. Note that Cursors, unlike iterators can continue to iterate past the
-     * "end"
+     * "end".
      *
      * If the former position no longer exists, but it is safe to continue iterating, the
      * following call to next() will return the next closest position in the direction of the
      * scan, if any.
      *
      * This handles restoring after either save() or SeekableRecordCursor::saveUnpositioned().
+     *
+     * 'tolerateCappedRepositioning' allows repositioning a capped cursor, which is useful for
+     * range writes.
      */
-    virtual bool restore() = 0;
+    virtual bool restore(bool tolerateCappedRepositioning = true) = 0;
 
     /**
      * Detaches from the OperationContext and releases any storage-engine state.
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
index 1975841524b..3a79a727fe4 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
@@ -711,7 +711,7 @@ public:
         }
     }
 
-    bool restore() final {
+    bool restore(bool tolerateCappedRepositioning = true) final {
         // We can't use the CursorCache since this cursor needs a special config string.
         WT_SESSION* session = WiredTigerRecoveryUnit::get(_opCtx)->getSession()->getSession();
 
@@ -2213,7 +2213,7 @@ void WiredTigerRecordStoreCursorBase::saveUnpositioned() {
     _lastReturnedId = RecordId();
 }
 
-bool WiredTigerRecordStoreCursorBase::restore() {
+bool WiredTigerRecordStoreCursorBase::restore(bool tolerateCappedRepositioning) {
     if (_rs._isOplog && _forward) {
         auto wtRu = WiredTigerRecoveryUnit::get(_opCtx);
         wtRu->setIsOplogReader();
@@ -2246,7 +2246,13 @@ bool WiredTigerRecordStoreCursorBase::restore() {
     RecordId id;
     if (ret == WT_NOTFOUND) {
         _eof = true;
-        return !_rs._isCapped;
+        if (_rs._isCapped && !tolerateCappedRepositioning) {
+            // Capped read collscans do not tolerate cursor repositioning.
+            // By contrast, write collscans on a clustered collection like TTL deletion
+            // tolerate cursor repositioning like normal collections.
+            return false;
+        }
+        return true;
     }
     invariantWTOK(ret);
     id = getKey(c);
@@ -2254,11 +2260,12 @@ bool WiredTigerRecordStoreCursorBase::restore() {
     if (cmp == 0)
         return true;  // Landed right where we left off.
 
-    if (_rs._isCapped) {
-        // Document was removed by capped collection operations. It is important that we error out
-        // in this case so that consumers don't silently get 'holes' when scanning capped
-        // collections. We don't make this guarantee for normal collections so it is ok to skip
-        // ahead in that case.
+    if (_rs._isCapped && !tolerateCappedRepositioning) {
+        // The cursor has been repositioned as it was sitting on a document that has been
+        // removed by capped collection deletion. It is important that we error out in this case
+        // so that consumers don't silently get 'holes' when scanning capped collections.
+        // We don't make this guarantee for normal collections or for write operations like
+        // capped TTL deletion so it is ok to skip ahead in that case.
         _eof = true;
         return false;
     }
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
index d92ba372fc7..8df216c4da4 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
@@ -402,7 +402,7 @@ public:
 
     void saveUnpositioned();
 
-    bool restore();
+    bool restore(bool tolerateCappedRepositioning = true);
 
     void detachFromOperationContext();