From 6da8828226321ef96eb05c79f3899b759844e02b Mon Sep 17 00:00:00 2001 From: Cheahuychou Mao Date: Wed, 16 Mar 2022 18:44:11 +0000 Subject: SERVER-64415 Make MigrationChunkClonerSourceLegacy not add pre/post image opTime to the session migration queue --- .../db/s/migration_chunk_cloner_source_legacy.cpp | 50 +++++++++------------- .../db/s/migration_chunk_cloner_source_legacy.h | 5 +-- .../db/s/session_catalog_migration_source.cpp | 21 ++------- src/mongo/db/s/session_catalog_migration_source.h | 2 +- 4 files changed, 26 insertions(+), 52 deletions(-) diff --git a/src/mongo/db/s/migration_chunk_cloner_source_legacy.cpp b/src/mongo/db/s/migration_chunk_cloner_source_legacy.cpp index d8ab0893eb6..27f56dc97d9 100644 --- a/src/mongo/db/s/migration_chunk_cloner_source_legacy.cpp +++ b/src/mongo/db/s/migration_chunk_cloner_source_legacy.cpp @@ -136,16 +136,11 @@ public: LogOpForShardingHandler(MigrationChunkClonerSourceLegacy* cloner, const BSONObj& idObj, const char op, - const repl::OpTime& opTime, - const repl::OpTime& prePostImageOpTime) - : _cloner(cloner), - _idObj(idObj.getOwned()), - _op(op), - _opTime(opTime), - _prePostImageOpTime(prePostImageOpTime) {} + const repl::OpTime& opTime) + : _cloner(cloner), _idObj(idObj.getOwned()), _op(op), _opTime(opTime) {} void commit(boost::optional) override { - _cloner->_addToTransferModsQueue(_idObj, _op, _opTime, _prePostImageOpTime); + _cloner->_addToTransferModsQueue(_idObj, _op, _opTime); _cloner->_decrementOutstandingOperationTrackRequests(); } @@ -158,7 +153,6 @@ private: const BSONObj _idObj; const char _op; const repl::OpTime _opTime; - const repl::OpTime _prePostImageOpTime; }; void LogTransactionOperationsForShardingHandler::commit(boost::optional) { @@ -225,7 +219,7 @@ void LogTransactionOperationsForShardingHandler::commit(boost::optional_addToTransferModsQueue(idElement.wrap(), getOpCharForCrudOpType(opType), {}, {}); + cloner->_addToTransferModsQueue(idElement.wrap(), getOpCharForCrudOpType(opType), {}); } } @@ -446,11 +440,11 @@ void MigrationChunkClonerSourceLegacy::onInsertOp(OperationContext* opCtx, } if (opCtx->getTxnNumber()) { - opCtx->recoveryUnit()->registerChange(std::make_unique( - this, idElement.wrap(), 'i', opTime, repl::OpTime())); + opCtx->recoveryUnit()->registerChange( + std::make_unique(this, idElement.wrap(), 'i', opTime)); } else { - opCtx->recoveryUnit()->registerChange(std::make_unique( - this, idElement.wrap(), 'i', repl::OpTime(), repl::OpTime())); + opCtx->recoveryUnit()->registerChange( + std::make_unique(this, idElement.wrap(), 'i', repl::OpTime())); } } @@ -489,18 +483,18 @@ void MigrationChunkClonerSourceLegacy::onUpdateOp(OperationContext* opCtx, } if (opCtx->getTxnNumber()) { - opCtx->recoveryUnit()->registerChange(std::make_unique( - this, idElement.wrap(), 'u', opTime, prePostImageOpTime)); + opCtx->recoveryUnit()->registerChange( + std::make_unique(this, idElement.wrap(), 'u', opTime)); } else { - opCtx->recoveryUnit()->registerChange(std::make_unique( - this, idElement.wrap(), 'u', repl::OpTime(), repl::OpTime())); + opCtx->recoveryUnit()->registerChange( + std::make_unique(this, idElement.wrap(), 'u', repl::OpTime())); } } void MigrationChunkClonerSourceLegacy::onDeleteOp(OperationContext* opCtx, const BSONObj& deletedDocId, const repl::OpTime& opTime, - const repl::OpTime& preImageOpTime) { + const repl::OpTime&) { dassert(opCtx->lockState()->isCollectionLockedForMode(_args.getNss(), MODE_IX)); BSONElement idElement = deletedDocId["_id"]; @@ -519,11 +513,11 @@ void MigrationChunkClonerSourceLegacy::onDeleteOp(OperationContext* opCtx, } if (opCtx->getTxnNumber()) { - opCtx->recoveryUnit()->registerChange(std::make_unique( - this, idElement.wrap(), 'd', opTime, preImageOpTime)); + opCtx->recoveryUnit()->registerChange( + std::make_unique(this, idElement.wrap(), 'd', opTime)); } else { - opCtx->recoveryUnit()->registerChange(std::make_unique( - this, idElement.wrap(), 'd', repl::OpTime(), repl::OpTime())); + opCtx->recoveryUnit()->registerChange( + std::make_unique(this, idElement.wrap(), 'd', repl::OpTime())); } } @@ -537,11 +531,9 @@ void MigrationChunkClonerSourceLegacy::_addToSessionMigrationOptimeQueue( } } -void MigrationChunkClonerSourceLegacy::_addToTransferModsQueue( - const BSONObj& idObj, - const char op, - const repl::OpTime& opTime, - const repl::OpTime& prePostImageOpTime) { +void MigrationChunkClonerSourceLegacy::_addToTransferModsQueue(const BSONObj& idObj, + const char op, + const repl::OpTime& opTime) { switch (op) { case 'd': { stdx::lock_guard sl(_mutex); @@ -562,8 +554,6 @@ void MigrationChunkClonerSourceLegacy::_addToTransferModsQueue( MONGO_UNREACHABLE; } - _addToSessionMigrationOptimeQueue( - prePostImageOpTime, SessionCatalogMigrationSource::EntryAtOpTimeType::kRetryableWrite); _addToSessionMigrationOptimeQueue( opTime, SessionCatalogMigrationSource::EntryAtOpTimeType::kRetryableWrite); } diff --git a/src/mongo/db/s/migration_chunk_cloner_source_legacy.h b/src/mongo/db/s/migration_chunk_cloner_source_legacy.h index f7c29efc16b..d794bb45372 100644 --- a/src/mongo/db/s/migration_chunk_cloner_source_legacy.h +++ b/src/mongo/db/s/migration_chunk_cloner_source_legacy.h @@ -261,10 +261,7 @@ private: * part of a part of a chunk being migrated. In doing so, this the method also removes the * corresponding operation track request from the operation track requests queue. */ - void _addToTransferModsQueue(const BSONObj& idObj, - char op, - const repl::OpTime& opTime, - const repl::OpTime& prePostImageOpTime); + void _addToTransferModsQueue(const BSONObj& idObj, char op, const repl::OpTime& opTime); /** * Adds an operation to the outstanding operation track requests. Returns false if the cloner diff --git a/src/mongo/db/s/session_catalog_migration_source.cpp b/src/mongo/db/s/session_catalog_migration_source.cpp index 3d378f3a8ed..75b22f61342 100644 --- a/src/mongo/db/s/session_catalog_migration_source.cpp +++ b/src/mongo/db/s/session_catalog_migration_source.cpp @@ -456,7 +456,7 @@ bool SessionCatalogMigrationSource::_fetchNextNewWriteOplog(OperationContext* op stdx::lock_guard lk(_newOplogMutex); if (_lastFetchedNewWriteOplogImage) { // When `_lastFetchedNewWriteOplogImage` is set, it means we found an oplog entry with - // `needsRetryImage`. At this step, we've already returned the image document, but we + // a pre/post image. At this step, we've already returned the image oplog entry, but we // have yet to return the original oplog entry stored in `_lastFetchedNewWriteOplog`. We // will unset this value and return such that the next call to `getLastFetchedOplog` // will return `_lastFetchedNewWriteOplog`. @@ -492,28 +492,15 @@ bool SessionCatalogMigrationSource::_fetchNextNewWriteOplog(OperationContext* op opCtx->getServiceContext()->getFastClockSource()->now()); } - boost::optional forgedNoopImage; - if (newWriteOplogEntry.getNeedsRetryImage()) { - // Generate the image outside of the mutex. Assign it atomically with the actual oplog - // entry. - forgedNoopImage = forgeNoopEntryFromImageCollection(opCtx, newWriteOplogEntry); - if (forgedNoopImage == boost::none) { - // No pre/post image was found. Defensively strip the `needsRetryImage` value to remove - // any notion this operation was a retryable findAndModify. If the request is retried on - // the destination, it will surface an error to the user. - auto mutableOplog = fassert(5676404, repl::MutableOplogEntry::parse(newWriteOplogDoc)); - mutableOplog.setNeedsRetryImage(boost::none); - newWriteOplogEntry = repl::OplogEntry(mutableOplog.toBSON()); - } - } + auto imageNoopOplogEntry = fetchPrePostImageOplog(opCtx, &newWriteOplogEntry); { stdx::lock_guard lk(_newOplogMutex); _lastFetchedNewWriteOplog = newWriteOplogEntry; _newWriteOpTimeList.pop_front(); - if (forgedNoopImage) { - _lastFetchedNewWriteOplogImage = forgedNoopImage; + if (imageNoopOplogEntry) { + _lastFetchedNewWriteOplogImage = imageNoopOplogEntry; } } diff --git a/src/mongo/db/s/session_catalog_migration_source.h b/src/mongo/db/s/session_catalog_migration_source.h index d080836921f..3603ddfb055 100644 --- a/src/mongo/db/s/session_catalog_migration_source.h +++ b/src/mongo/db/s/session_catalog_migration_source.h @@ -283,7 +283,7 @@ private: // Used to store the last fetched oplog from _newWriteTsList. boost::optional _lastFetchedNewWriteOplog; - // Used to store an image when `_lastFetchedNewWriteOplog` has a `needsRetryImage` field. + // Used to store an image for `_lastFetchedNewWriteOplog` if there is one. boost::optional _lastFetchedNewWriteOplogImage; // Stores the current state. -- cgit v1.2.1