diff options
author | Andrew Shuvalov <andrew.shuvalov@mongodb.com> | 2021-05-18 22:55:06 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-05-18 23:27:52 +0000 |
commit | 22d98bf4cf5c12497d57fe712db0c9440e7ceefb (patch) | |
tree | 53a5452b1033e5f0a9cd7f7077220542d551b94b | |
parent | d6c259d22efe9c5aba86f796a6b352f164c0ee36 (diff) | |
download | mongo-22d98bf4cf5c12497d57fe712db0c9440e7ceefb.tar.gz |
SERVER-56373: BACKPORT-8899 Refactor Oplog onDelete callback to accomodate new preImage arg
-rw-r--r-- | src/mongo/db/auth/auth_op_observer.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/auth/auth_op_observer.h | 3 | ||||
-rw-r--r-- | src/mongo/db/auth/auth_op_observer_test.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_impl.cpp | 9 | ||||
-rw-r--r-- | src/mongo/db/free_mon/free_mon_op_observer.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/free_mon/free_mon_op_observer.h | 3 | ||||
-rw-r--r-- | src/mongo/db/op_observer.h | 18 | ||||
-rw-r--r-- | src/mongo/db/op_observer_impl.cpp | 9 | ||||
-rw-r--r-- | src/mongo/db/op_observer_impl.h | 3 | ||||
-rw-r--r-- | src/mongo/db/op_observer_impl_test.cpp | 24 | ||||
-rw-r--r-- | src/mongo/db/op_observer_noop.h | 3 | ||||
-rw-r--r-- | src/mongo/db/op_observer_registry.h | 5 | ||||
-rw-r--r-- | src/mongo/db/repl/oplog_entry.idl | 6 | ||||
-rw-r--r-- | src/mongo/db/repl/sync_tail_test_fixture.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/repl/sync_tail_test_fixture.h | 3 | ||||
-rw-r--r-- | src/mongo/db/s/config_server_op_observer.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/s/config_server_op_observer.h | 3 | ||||
-rw-r--r-- | src/mongo/db/s/shard_server_op_observer.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/s/shard_server_op_observer.h | 3 |
19 files changed, 64 insertions, 57 deletions
diff --git a/src/mongo/db/auth/auth_op_observer.cpp b/src/mongo/db/auth/auth_op_observer.cpp index df979414ba6..378c3052645 100644 --- a/src/mongo/db/auth/auth_op_observer.cpp +++ b/src/mongo/db/auth/auth_op_observer.cpp @@ -85,8 +85,7 @@ void AuthOpObserver::onDelete(OperationContext* opCtx, const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) { + const OplogDeleteEntryArgs& args) { auto& documentKey = documentKeyDecoration(opCtx); invariant(!documentKey.isEmpty()); AuthorizationManager::get(opCtx->getServiceContext()) diff --git a/src/mongo/db/auth/auth_op_observer.h b/src/mongo/db/auth/auth_op_observer.h index 3e91a9c9884..d5c5ec8f85b 100644 --- a/src/mongo/db/auth/auth_op_observer.h +++ b/src/mongo/db/auth/auth_op_observer.h @@ -89,8 +89,7 @@ public: const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) final; + const OplogDeleteEntryArgs& args) final; void onInternalOpMessage(OperationContext* opCtx, const NamespaceString& nss, diff --git a/src/mongo/db/auth/auth_op_observer_test.cpp b/src/mongo/db/auth/auth_op_observer_test.cpp index 1168ae1836d..af4e8925666 100644 --- a/src/mongo/db/auth/auth_op_observer_test.cpp +++ b/src/mongo/db/auth/auth_op_observer_test.cpp @@ -137,9 +137,9 @@ TEST_F(AuthOpObserverTest, MultipleAboutToDeleteAndOnDelete) { AutoGetDb autoDb(opCtx.get(), nss.db(), MODE_X); WriteUnitOfWork wunit(opCtx.get()); opObserver.aboutToDelete(opCtx.get(), nss, BSON("_id" << 1)); - opObserver.onDelete(opCtx.get(), nss, uuid, {}, false, {}); + opObserver.onDelete(opCtx.get(), nss, uuid, {}, {}); opObserver.aboutToDelete(opCtx.get(), nss, BSON("_id" << 1)); - opObserver.onDelete(opCtx.get(), nss, uuid, {}, false, {}); + opObserver.onDelete(opCtx.get(), nss, uuid, {}, {}); } DEATH_TEST_F(AuthOpObserverTest, AboutToDeleteMustPreceedOnDelete, "invariant") { @@ -147,7 +147,7 @@ DEATH_TEST_F(AuthOpObserverTest, AboutToDeleteMustPreceedOnDelete, "invariant") auto opCtx = cc().makeOperationContext(); opCtx->swapLockState(stdx::make_unique<LockerNoop>()); NamespaceString nss = {"test", "coll"}; - opObserver.onDelete(opCtx.get(), nss, {}, {}, false, {}); + opObserver.onDelete(opCtx.get(), nss, {}, {}, {}); } DEATH_TEST_F(AuthOpObserverTest, EachOnDeleteRequiresAboutToDelete, "invariant") { @@ -156,8 +156,8 @@ DEATH_TEST_F(AuthOpObserverTest, EachOnDeleteRequiresAboutToDelete, "invariant") opCtx->swapLockState(stdx::make_unique<LockerNoop>()); NamespaceString nss = {"test", "coll"}; opObserver.aboutToDelete(opCtx.get(), nss, {}); - opObserver.onDelete(opCtx.get(), nss, {}, {}, false, {}); - opObserver.onDelete(opCtx.get(), nss, {}, {}, false, {}); + opObserver.onDelete(opCtx.get(), nss, {}, {}, {}); + opObserver.onDelete(opCtx.get(), nss, {}, {}, {}); } } // namespace diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp index c6498fc3afb..d8b774ceabd 100644 --- a/src/mongo/db/catalog/collection_impl.cpp +++ b/src/mongo/db/catalog/collection_impl.cpp @@ -671,8 +671,13 @@ void CollectionImpl::deleteDocument(OperationContext* opCtx, _indexCatalog->unindexRecord(opCtx, doc.value(), loc, noWarn, &keysDeleted); _recordStore->deleteRecord(opCtx, loc); - getGlobalServiceContext()->getOpObserver()->onDelete( - opCtx, ns(), uuid(), stmtId, fromMigrate, deletedDoc); + OpObserver::OplogDeleteEntryArgs deleteArgs; + if (deletedDoc) { + deleteArgs.deletedDoc = &(deletedDoc.get()); + } + deleteArgs.fromMigrate = fromMigrate; + + getGlobalServiceContext()->getOpObserver()->onDelete(opCtx, ns(), uuid(), stmtId, deleteArgs); if (opDebug) { opDebug->additiveMetrics.incrementKeysDeleted(keysDeleted); diff --git a/src/mongo/db/free_mon/free_mon_op_observer.cpp b/src/mongo/db/free_mon/free_mon_op_observer.cpp index 29e380c8baa..73861cc31d9 100644 --- a/src/mongo/db/free_mon/free_mon_op_observer.cpp +++ b/src/mongo/db/free_mon/free_mon_op_observer.cpp @@ -134,8 +134,7 @@ void FreeMonOpObserver::onDelete(OperationContext* opCtx, const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) { + const OplogDeleteEntryArgs& args) { if (nss != NamespaceString::kServerConfigurationNamespace) { return; } diff --git a/src/mongo/db/free_mon/free_mon_op_observer.h b/src/mongo/db/free_mon/free_mon_op_observer.h index 7b17ebd014b..e3e591973fa 100644 --- a/src/mongo/db/free_mon/free_mon_op_observer.h +++ b/src/mongo/db/free_mon/free_mon_op_observer.h @@ -89,8 +89,7 @@ public: const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) final; + const OplogDeleteEntryArgs& args) final; void onInternalOpMessage(OperationContext* opCtx, const NamespaceString& nss, diff --git a/src/mongo/db/op_observer.h b/src/mongo/db/op_observer.h index 85847e698ef..b5b4a992246 100644 --- a/src/mongo/db/op_observer.h +++ b/src/mongo/db/op_observer.h @@ -125,20 +125,26 @@ public: virtual void aboutToDelete(OperationContext* opCtx, const NamespaceString& nss, const BSONObj& doc) = 0; + + struct OplogDeleteEntryArgs { + const BSONObj* deletedDoc = nullptr; + // indicates whether the delete was induced by a chunk migration, and + // so should be ignored by the user as an internal maintenance operation and not a + // real delete. + bool fromMigrate = false; + bool preImageRecordingEnabledForCollection = false; + }; + /** * Handles logging before document is deleted. * - * "ns" name of the collection from which deleteState.idDoc will be deleted. - * "fromMigrate" indicates whether the delete was induced by a chunk migration, and - * so should be ignored by the user as an internal maintenance operation and not a - * real delete. + * "nss" name of the collection from which deleteState.idDoc will be deleted. */ virtual void onDelete(OperationContext* opCtx, const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) = 0; + const OplogDeleteEntryArgs& args) = 0; /** * Logs a no-op with "msgObj" in the o field into oplog. * diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp index d5217c1e535..9254605f578 100644 --- a/src/mongo/db/op_observer_impl.cpp +++ b/src/mongo/db/op_observer_impl.cpp @@ -603,8 +603,7 @@ void OpObserverImpl::onDelete(OperationContext* opCtx, const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) { + const OplogDeleteEntryArgs& args) { auto& documentKey = documentKeyDecoration(opCtx); invariant(!documentKey.isEmpty()); @@ -617,7 +616,9 @@ void OpObserverImpl::onDelete(OperationContext* opCtx, auto operation = OplogEntry::makeDeleteOperation(nss, uuid, documentKey); txnParticipant.addTransactionOperation(opCtx, operation); } else { - opTime = replLogDelete(opCtx, nss, uuid, stmtId, fromMigrate, deletedDoc); + boost::optional<BSONObj> deletedDoc = + args.deletedDoc ? boost::optional<BSONObj>(*(args.deletedDoc)) : boost::none; + opTime = replLogDelete(opCtx, nss, uuid, stmtId, args.fromMigrate, deletedDoc); SessionTxnRecord sessionTxnRecord; sessionTxnRecord.setLastWriteOpTime(opTime.writeOpTime); sessionTxnRecord.setLastWriteDate(opTime.wallClockTime); @@ -625,7 +626,7 @@ void OpObserverImpl::onDelete(OperationContext* opCtx, } if (nss != NamespaceString::kSessionTransactionsTableNamespace) { - if (!fromMigrate) { + if (!args.fromMigrate) { shardObserveDeleteOp(opCtx, nss, documentKey, diff --git a/src/mongo/db/op_observer_impl.h b/src/mongo/db/op_observer_impl.h index bebd01101fc..9fb50fda884 100644 --- a/src/mongo/db/op_observer_impl.h +++ b/src/mongo/db/op_observer_impl.h @@ -82,8 +82,7 @@ public: const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) final; + const OplogDeleteEntryArgs& args) final; void onInternalOpMessage(OperationContext* opCtx, const NamespaceString& nss, const boost::optional<UUID> uuid, diff --git a/src/mongo/db/op_observer_impl_test.cpp b/src/mongo/db/op_observer_impl_test.cpp index ec10e37bd44..27339d2d086 100644 --- a/src/mongo/db/op_observer_impl_test.cpp +++ b/src/mongo/db/op_observer_impl_test.cpp @@ -524,9 +524,9 @@ TEST_F(OpObserverTest, MultipleAboutToDeleteAndOnDelete) { AutoGetDb autoDb(opCtx.get(), nss.db(), MODE_X); WriteUnitOfWork wunit(opCtx.get()); opObserver.aboutToDelete(opCtx.get(), nss, BSON("_id" << 1)); - opObserver.onDelete(opCtx.get(), nss, uuid, {}, false, {}); + opObserver.onDelete(opCtx.get(), nss, uuid, {}, {}); opObserver.aboutToDelete(opCtx.get(), nss, BSON("_id" << 1)); - opObserver.onDelete(opCtx.get(), nss, uuid, {}, false, {}); + opObserver.onDelete(opCtx.get(), nss, uuid, {}, {}); } DEATH_TEST_F(OpObserverTest, AboutToDeleteMustPreceedOnDelete, "invariant") { @@ -534,7 +534,7 @@ DEATH_TEST_F(OpObserverTest, AboutToDeleteMustPreceedOnDelete, "invariant") { auto opCtx = cc().makeOperationContext(); opCtx->swapLockState(stdx::make_unique<LockerNoop>()); NamespaceString nss = {"test", "coll"}; - opObserver.onDelete(opCtx.get(), nss, {}, {}, false, {}); + opObserver.onDelete(opCtx.get(), nss, {}, {}, {}); } DEATH_TEST_F(OpObserverTest, EachOnDeleteRequiresAboutToDelete, "invariant") { @@ -543,8 +543,8 @@ DEATH_TEST_F(OpObserverTest, EachOnDeleteRequiresAboutToDelete, "invariant") { opCtx->swapLockState(stdx::make_unique<LockerNoop>()); NamespaceString nss = {"test", "coll"}; opObserver.aboutToDelete(opCtx.get(), nss, {}); - opObserver.onDelete(opCtx.get(), nss, {}, {}, false, {}); - opObserver.onDelete(opCtx.get(), nss, {}, {}, false, {}); + opObserver.onDelete(opCtx.get(), nss, {}, {}, {}); + opObserver.onDelete(opCtx.get(), nss, {}, {}, {}); } DEATH_TEST_F(OpObserverTest, @@ -724,7 +724,7 @@ TEST_F(OpObserverTransactionTest, TransactionalPrepareTest) { nss1, BSON("_id" << 0 << "data" << "x")); - opObserver().onDelete(opCtx(), nss1, uuid1, 0, false, boost::none); + opObserver().onDelete(opCtx(), nss1, uuid1, 0, {}); { Lock::GlobalLock lk(opCtx(), MODE_IX); @@ -1233,12 +1233,12 @@ TEST_F(OpObserverTransactionTest, TransactionalDeleteTest) { nss1, BSON("_id" << 0 << "data" << "x")); - opObserver().onDelete(opCtx(), nss1, uuid1, 0, false, boost::none); + opObserver().onDelete(opCtx(), nss1, uuid1, 0, {}); opObserver().aboutToDelete(opCtx(), nss2, BSON("_id" << 1 << "data" << "y")); - opObserver().onDelete(opCtx(), nss2, uuid2, 0, false, boost::none); + opObserver().onDelete(opCtx(), nss2, uuid2, 0, {}); opObserver().onUnpreparedTransactionCommit( opCtx(), txnParticipant.retrieveCompletedTransactionOperations(opCtx())); auto oplogEntry = getSingleOplogEntry(opCtx()); @@ -1451,12 +1451,12 @@ TEST_F(OpObserverMultiEntryTransactionTest, TransactionalDeleteTest) { nss1, BSON("_id" << 0 << "data" << "x")); - opObserver().onDelete(opCtx(), nss1, uuid1, 0, false, boost::none); + opObserver().onDelete(opCtx(), nss1, uuid1, 0, {}); opObserver().aboutToDelete(opCtx(), nss2, BSON("_id" << 1 << "data" << "y")); - opObserver().onDelete(opCtx(), nss2, uuid2, 0, false, boost::none); + opObserver().onDelete(opCtx(), nss2, uuid2, 0, {}); opObserver().onUnpreparedTransactionCommit( opCtx(), txnParticipant.retrieveCompletedTransactionOperations(opCtx())); auto oplogEntryObjs = getNOplogEntries(opCtx(), 2); @@ -1665,12 +1665,12 @@ TEST_F(OpObserverMultiEntryTransactionTest, TransactionalDeletePrepareTest) { nss1, BSON("_id" << 0 << "data" << "x")); - opObserver().onDelete(opCtx(), nss1, uuid1, 0, false, boost::none); + opObserver().onDelete(opCtx(), nss1, uuid1, 0, {}); opObserver().aboutToDelete(opCtx(), nss2, BSON("_id" << 1 << "data" << "y")); - opObserver().onDelete(opCtx(), nss2, uuid2, 0, false, boost::none); + opObserver().onDelete(opCtx(), nss2, uuid2, 0, {}); repl::OpTime prepareOpTime; { diff --git a/src/mongo/db/op_observer_noop.h b/src/mongo/db/op_observer_noop.h index 6c72a04aa25..f3543e3e1ae 100644 --- a/src/mongo/db/op_observer_noop.h +++ b/src/mongo/db/op_observer_noop.h @@ -76,8 +76,7 @@ public: const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) override {} + const OplogDeleteEntryArgs& args) override {} void onInternalOpMessage(OperationContext* opCtx, const NamespaceString& nss, const boost::optional<UUID> uuid, diff --git a/src/mongo/db/op_observer_registry.h b/src/mongo/db/op_observer_registry.h index a930b9aa608..ba1ef6ba91e 100644 --- a/src/mongo/db/op_observer_registry.h +++ b/src/mongo/db/op_observer_registry.h @@ -133,11 +133,10 @@ public: const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) override { + const OplogDeleteEntryArgs& args) override { ReservedTimes times{opCtx}; for (auto& o : _observers) - o->onDelete(opCtx, nss, uuid, stmtId, fromMigrate, deletedDoc); + o->onDelete(opCtx, nss, uuid, stmtId, args); } void onInternalOpMessage(OperationContext* const opCtx, diff --git a/src/mongo/db/repl/oplog_entry.idl b/src/mongo/db/repl/oplog_entry.idl index 241f77c0c5d..96d2934f033 100644 --- a/src/mongo/db/repl/oplog_entry.idl +++ b/src/mongo/db/repl/oplog_entry.idl @@ -148,3 +148,9 @@ structs: optional: true description: "The optime of another oplog entry that contains the document after an update was applied." + needsRetryImage: + type: RetryImage + optional: true + description: "Identifies whether a secondary should store a pre-image or post-image + associated with this oplog entry." + diff --git a/src/mongo/db/repl/sync_tail_test_fixture.cpp b/src/mongo/db/repl/sync_tail_test_fixture.cpp index 1d0026164f8..fe36dd321be 100644 --- a/src/mongo/db/repl/sync_tail_test_fixture.cpp +++ b/src/mongo/db/repl/sync_tail_test_fixture.cpp @@ -72,12 +72,13 @@ void SyncTailOpObserver::onDelete(OperationContext* opCtx, const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) { + const OplogDeleteEntryArgs& args) { if (!onDeleteFn) { return; } - onDeleteFn(opCtx, nss, uuid, stmtId, fromMigrate, deletedDoc); + boost::optional<BSONObj> deletedDoc = + args.deletedDoc ? boost::optional<BSONObj>(*(args.deletedDoc)) : boost::none; + onDeleteFn(opCtx, nss, uuid, stmtId, args.fromMigrate, deletedDoc); } void SyncTailOpObserver::onCreateCollection(OperationContext* opCtx, diff --git a/src/mongo/db/repl/sync_tail_test_fixture.h b/src/mongo/db/repl/sync_tail_test_fixture.h index 3c64370c4b9..31d7f5be90f 100644 --- a/src/mongo/db/repl/sync_tail_test_fixture.h +++ b/src/mongo/db/repl/sync_tail_test_fixture.h @@ -68,8 +68,7 @@ public: const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) override; + const OplogDeleteEntryArgs& args) override; /** * Called when SyncTail creates a collection. diff --git a/src/mongo/db/s/config_server_op_observer.cpp b/src/mongo/db/s/config_server_op_observer.cpp index 6019b36a338..56d53dd535d 100644 --- a/src/mongo/db/s/config_server_op_observer.cpp +++ b/src/mongo/db/s/config_server_op_observer.cpp @@ -47,8 +47,7 @@ void ConfigServerOpObserver::onDelete(OperationContext* opCtx, const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) { + const OplogDeleteEntryArgs& args) { if (nss == VersionType::ConfigNS) { if (!repl::ReplicationCoordinator::get(opCtx)->getMemberState().rollback()) { uasserted(40302, "cannot delete config.version document while in --configsvr mode"); diff --git a/src/mongo/db/s/config_server_op_observer.h b/src/mongo/db/s/config_server_op_observer.h index bf804fbf6c4..93d4ef813d2 100644 --- a/src/mongo/db/s/config_server_op_observer.h +++ b/src/mongo/db/s/config_server_op_observer.h @@ -89,8 +89,7 @@ public: const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) override; + const OplogDeleteEntryArgs& args) override; void onInternalOpMessage(OperationContext* opCtx, const NamespaceString& nss, diff --git a/src/mongo/db/s/shard_server_op_observer.cpp b/src/mongo/db/s/shard_server_op_observer.cpp index 980a4be7865..4f54a7d7f92 100644 --- a/src/mongo/db/s/shard_server_op_observer.cpp +++ b/src/mongo/db/s/shard_server_op_observer.cpp @@ -350,8 +350,7 @@ void ShardServerOpObserver::onDelete(OperationContext* opCtx, const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) { + const OplogDeleteEntryArgs& args) { auto& documentKey = getDocumentKey(opCtx); if (nss == NamespaceString::kShardConfigCollectionsNamespace) { diff --git a/src/mongo/db/s/shard_server_op_observer.h b/src/mongo/db/s/shard_server_op_observer.h index 339f5968bf8..52c35c5f1ce 100644 --- a/src/mongo/db/s/shard_server_op_observer.h +++ b/src/mongo/db/s/shard_server_op_observer.h @@ -90,8 +90,7 @@ public: const NamespaceString& nss, OptionalCollectionUUID uuid, StmtId stmtId, - bool fromMigrate, - const boost::optional<BSONObj>& deletedDoc) override; + const OplogDeleteEntryArgs& args) override; void onInternalOpMessage(OperationContext* opCtx, const NamespaceString& nss, |