diff options
-rw-r--r-- | src/mongo/db/s/migration_source_manager.cpp | 85 |
1 files changed, 49 insertions, 36 deletions
diff --git a/src/mongo/db/s/migration_source_manager.cpp b/src/mongo/db/s/migration_source_manager.cpp index 28c7ad17c9a..24f30e32b0d 100644 --- a/src/mongo/db/s/migration_source_manager.cpp +++ b/src/mongo/db/s/migration_source_manager.cpp @@ -520,48 +520,62 @@ Status MigrationSourceManager::commitChunkMetadataOnConfig(OperationContext* opC << redact(status)}); } - // Do a best effort attempt to incrementally refresh the metadata before leaving the critical - // section. It is okay if the refresh fails because that will cause the metadata to be cleared - // and subsequent callers will try to do a full refresh. - ChunkVersion unusedShardVersion; - Status refreshStatus = - ShardingState::get(opCtx)->refreshMetadataNow(opCtx, getNss(), &unusedShardVersion); - - if (!refreshStatus.isOK()) { - AutoGetCollection autoColl(opCtx, getNss(), MODE_IX, MODE_X); + // Because the CatalogCache's WithRefresh methods (on which forceShardFilteringMetadataRefresh + // depends) are not causally consistent, we need to perform up to two refresh rounds if refresh + // returns that the shard still owns the chunk + ChunkVersion collectionVersionAfterRefresh; - CollectionShardingState::get(opCtx, getNss())->refreshMetadata(opCtx, nullptr); + for (int retriesLeft = 2; retriesLeft > 0; --retriesLeft) { + ChunkVersion unusedShardVersion; + Status refreshStatus = + ShardingState::get(opCtx)->refreshMetadataNow(opCtx, getNss(), &unusedShardVersion); - log() << "Failed to refresh metadata after a " - << (migrationCommitStatus.isOK() ? "failed commit attempt" : "successful commit") - << ". Metadata was cleared so it will get a full refresh when accessed again." - << causedBy(redact(refreshStatus)); + // If the refresh fails, there is no way to confirm whether the migration commit actually + // went through or not. Because of that, the collection's metadata is reset to UNSHARDED so + // that subsequent versioned requests will get StaleShardVersion and will retry the refresh. + if (!refreshStatus.isOK()) { + AutoGetCollection autoColl(opCtx, getNss(), MODE_IX, MODE_X); - // migrationCommitStatus may be OK or an error. The migration is considered a success at - // this point if the commit succeeded. The metadata refresh either occurred or the metadata - // was safely cleared. - return {migrationCommitStatus.code(), + CollectionShardingState::get(opCtx, getNss())->refreshMetadata(opCtx, nullptr); + + log() << "Failed to refresh metadata after a " + << (migrationCommitStatus.isOK() ? "failed commit attempt" : "successful commit") + << ". Metadata was cleared so it will get a full refresh when accessed again." + << causedBy(redact(refreshStatus)); + + return { + migrationCommitStatus.code(), str::stream() << "Orphaned range not cleaned up. Failed to refresh metadata after" " migration commit due to '" << refreshStatus.toString() << "', and commit failed due to '" << migrationCommitStatus.toString() << "'"}; - } + } - auto refreshedMetadata = [&] { - AutoGetCollection autoColl(opCtx, getNss(), MODE_IS); - return CollectionShardingState::get(opCtx, getNss())->getMetadata(); - }(); + auto refreshedMetadata = [&] { + AutoGetCollection autoColl(opCtx, getNss(), MODE_IS); + return CollectionShardingState::get(opCtx, getNss())->getMetadata(); + }(); - if (!refreshedMetadata) { - return {ErrorCodes::NamespaceNotSharded, - str::stream() << "Chunk move failed because collection '" << getNss().ns() - << "' is no longer sharded. The migration commit error was: " - << migrationCommitStatus.toString()}; - } + if (!refreshedMetadata) { + return {ErrorCodes::NamespaceNotSharded, + str::stream() << "Chunk move failed because collection '" << getNss().ns() + << "' is no longer sharded. The migration commit error was: " + << migrationCommitStatus.toString()}; + } + + // If after a successful refresh the metadata indicates that the node still owns the chunk, + // we must do one more refresh in order to ensure that the previous refresh round didn't + // join an already active catalog cache refresh and missed its own commit + if (!refreshedMetadata->keyBelongsToMe(_args.getMinKey())) { + collectionVersionAfterRefresh = refreshedMetadata->getCollVersion(); + break; + } + + if (retriesLeft) + continue; - if (refreshedMetadata->keyBelongsToMe(_args.getMinKey())) { // This condition may only happen if the migration commit has failed for any reason if (migrationCommitStatus.isOK()) { severe() << "The migration commit succeeded, but the new chunk placement was not " @@ -585,9 +599,11 @@ Status MigrationSourceManager::commitChunkMetadataOnConfig(OperationContext* opC << migrationCommitStatus.reason()}; } + invariant(collectionVersionAfterRefresh.isSet()); + // Migration succeeded log() << "Migration succeeded and updated collection version to " - << refreshedMetadata->getCollVersion(); + << collectionVersionAfterRefresh; MONGO_FAIL_POINT_PAUSE_WHILE_SET(hangBeforeLeavingCriticalSection); @@ -624,11 +640,8 @@ Status MigrationSourceManager::commitChunkMetadataOnConfig(OperationContext* opC if (!MONGO_FAIL_POINT(doNotRefreshRecipientAfterCommit)) { // Best-effort make the recipient refresh its routing table to the new collection version. - refreshRecipientRoutingTable(opCtx, - getNss(), - _args.getToShardId(), - _recipientHost, - refreshedMetadata->getCollVersion()); + refreshRecipientRoutingTable( + opCtx, getNss(), _args.getToShardId(), _recipientHost, collectionVersionAfterRefresh); } if (_args.getWaitForDelete()) { |