From f55559368fe6a51689ffc3c08bab4434cb1b2b99 Mon Sep 17 00:00:00 2001 From: David Storch Date: Tue, 14 Aug 2018 12:13:20 -0400 Subject: SERVER-16857 Delete MMAPv1 diskloc invalidations. - Removes of PlanStage::invalidate(). - Removes RecordCursor::invalidate() from the storage API. - Removes CursorManager::invalidateDocument(). --- .../db/storage/mobile/mobile_record_store.cpp | 5 +++ src/mongo/db/storage/record_store.h | 24 ++++------- .../db/storage/record_store_test_recorditer.cpp | 41 ++++++++++++++++++ .../db/storage/record_store_test_repairiter.cpp | 49 ---------------------- 4 files changed, 55 insertions(+), 64 deletions(-) (limited to 'src/mongo/db/storage') diff --git a/src/mongo/db/storage/mobile/mobile_record_store.cpp b/src/mongo/db/storage/mobile/mobile_record_store.cpp index 0111abb67e3..1ba8020a76f 100644 --- a/src/mongo/db/storage/mobile/mobile_record_store.cpp +++ b/src/mongo/db/storage/mobile/mobile_record_store.cpp @@ -112,6 +112,11 @@ public: restore(); boost::optional rec = next(); + if (rec && rec->id != id) { + // The record we found isn't the one the caller asked for. + return boost::none; + } + return rec; } diff --git a/src/mongo/db/storage/record_store.h b/src/mongo/db/storage/record_store.h index c0da914f734..097a56e3cbb 100644 --- a/src/mongo/db/storage/record_store.h +++ b/src/mongo/db/storage/record_store.h @@ -74,6 +74,9 @@ protected: /** * @see RecordStore::updateRecord + * + * TODO SERVER-36662: Without MMAPv1 diskloc invalidations, this interface is no longer necessary. + * This callback's entire role was to issue INVALIDATION_MUTATION notifications. */ class UpdateNotifier { public: @@ -109,12 +112,12 @@ enum ValidateCmdLevel : int { * inside that context. Any cursor acquired inside a transaction is invalid outside * of that transaction, instead use the save and restore methods to reestablish the cursor. * - * Any method other than invalidate and the save methods may throw WriteConflictException. If - * that happens, the cursor may not be used again until it has been saved and successfully - * restored. If next() or restore() throw a WCE the cursor's position will be the same as before - * the call (strong exception guarantee). All other methods leave the cursor in a valid state - * but with an unspecified position (basic exception guarantee). If any exception other than - * WCE is thrown, the cursor must be destroyed, which is guaranteed not to leak any resources. + * Any method other than the save method may throw WriteConflictException. If that happens, the + * cursor may not be used again until it has been saved and successfully restored. If next() or + * restore() throw a WCE the cursor's position will be the same as before the call (strong exception + * guarantee). All other methods leave the cursor in a valid state but with an unspecified position + * (basic exception guarantee). If any exception other than WCE is thrown, the cursor must be + * destroyed, which is guaranteed not to leak any resources. * * Any returned unowned BSON is only valid until the next call to any method on this * interface. @@ -194,15 +197,6 @@ public: */ virtual void reattachToOperationContext(OperationContext* opCtx) = 0; - /** - * Inform the cursor that this id is being invalidated. Must be called between save and restore. - * The opCtx is that of the operation causing the invalidation, not the opCtx using the cursor. - * - * WARNING: Storage engines other than MMAPv1 should use the default implementation, - * and not depend on this being called. - */ - virtual void invalidate(OperationContext* opCtx, const RecordId& id) {} - // // RecordFetchers // diff --git a/src/mongo/db/storage/record_store_test_recorditer.cpp b/src/mongo/db/storage/record_store_test_recorditer.cpp index 968dfbcb760..b7ff0e54ed4 100644 --- a/src/mongo/db/storage/record_store_test_recorditer.cpp +++ b/src/mongo/db/storage/record_store_test_recorditer.cpp @@ -443,5 +443,46 @@ TEST(RecordStoreTestHarness, SeekAfterEofAndContinue) { ASSERT(!cursor->next()); } +// seekExact() must return boost::none if the RecordId does not exist. +TEST(RecordStoreTestHarness, SeekExactForMissingRecordReturnsNone) { + const auto harnessHelper{newRecordStoreHarnessHelper()}; + auto recordStore = harnessHelper->newNonCappedRecordStore(); + ServiceContext::UniqueOperationContext opCtx{harnessHelper->newOperationContext()}; + + // Insert three records and remember their record ids. + const int nToInsert = 3; + RecordId recordIds[nToInsert]; + for (int i = 0; i < nToInsert; ++i) { + StringBuilder sb; + sb << "record " << i; + string data = sb.str(); + + WriteUnitOfWork uow{opCtx.get()}; + auto res = + recordStore->insertRecord(opCtx.get(), data.c_str(), data.size() + 1, Timestamp{}); + ASSERT_OK(res.getStatus()); + recordIds[i] = res.getValue(); + uow.commit(); + } + + // Delete the second record. + { + WriteUnitOfWork uow{opCtx.get()}; + recordStore->deleteRecord(opCtx.get(), recordIds[1]); + uow.commit(); + } + + // Seeking to the second record should now return boost::none, for both forward and reverse + // cursors. + for (bool direction : {true, false}) { + auto cursor = recordStore->getCursor(opCtx.get(), direction); + ASSERT(!cursor->seekExact(recordIds[1])); + } + + // Similarly, findRecord() should not find the deleted record. + RecordData outputData; + ASSERT_FALSE(recordStore->findRecord(opCtx.get(), recordIds[1], &outputData)); +} + } // namespace } // namespace mongo diff --git a/src/mongo/db/storage/record_store_test_repairiter.cpp b/src/mongo/db/storage/record_store_test_repairiter.cpp index a84e7dcab4d..aa8f221a538 100644 --- a/src/mongo/db/storage/record_store_test_repairiter.cpp +++ b/src/mongo/db/storage/record_store_test_repairiter.cpp @@ -119,54 +119,5 @@ TEST(RecordStoreTestHarness, GetIteratorForRepairNonEmpty) { } } -// Insert a single record. Create a repair iterator pointing to that single record. -// Then invalidate the record and ensure that the repair iterator responds correctly. -// See SERVER-16300. -TEST(RecordStoreTestHarness, GetIteratorForRepairInvalidateSingleton) { - const auto harnessHelper(newRecordStoreHarnessHelper()); - unique_ptr rs(harnessHelper->newNonCappedRecordStore()); - - { - ServiceContext::UniqueOperationContext opCtx(harnessHelper->newOperationContext()); - ASSERT_EQ(0, rs->numRecords(opCtx.get())); - } - - // Insert one record. - RecordId idToInvalidate; - { - ServiceContext::UniqueOperationContext opCtx(harnessHelper->newOperationContext()); - WriteUnitOfWork uow(opCtx.get()); - StatusWith res = rs->insertRecord(opCtx.get(), "some data", 10, Timestamp()); - ASSERT_OK(res.getStatus()); - idToInvalidate = res.getValue(); - uow.commit(); - } - - // Double-check that the record store has one record in it now. - { - ServiceContext::UniqueOperationContext opCtx(harnessHelper->newOperationContext()); - ASSERT_EQ(1, rs->numRecords(opCtx.get())); - } - - { - ServiceContext::UniqueOperationContext opCtx(harnessHelper->newOperationContext()); - auto cursor = rs->getCursorForRepair(opCtx.get()); - // returns NULL if getCursorForRepair is not supported - if (!cursor) { - return; - } - - // We should be pointing at the only record in the store. - - // Invalidate the record we're pointing at. - cursor->save(); - cursor->invalidate(opCtx.get(), idToInvalidate); - cursor->restore(); - - // Iterator should be EOF now because the only thing in the collection got deleted. - ASSERT(!cursor->next()); - } -} - } // namespace } // namespace mongo -- cgit v1.2.1