diff options
author | Gregory Wlodarek <gregory.wlodarek@mongodb.com> | 2022-04-15 20:09:22 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-04-15 23:21:18 +0000 |
commit | 4cbdf0b6db42e8ba867d17b56bd93358c9e6aee6 (patch) | |
tree | cf59d09f2e87aa733c90e4553edf2574565fb608 | |
parent | ad74e7495a16fbbb94b0bca4049acd534d369ada (diff) | |
download | mongo-4cbdf0b6db42e8ba867d17b56bd93358c9e6aee6.tar.gz |
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(); |