From 687611850d4cd6ea735104f421626a6f38ee3387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pol=20Pi=C3=B1ol=20Castuera?= Date: Tue, 26 May 2026 16:38:06 +0200 Subject: [PATCH] SERVER-125939 Make untrackUnshardedCollection commit to the shard catalog (#53132) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> GitOrigin-RevId: 7b279a5c9b7334e4488392330c9f857565acc555 --- ...unsharded_collection_with_orphaned_data.js | 14 +++- ...thoritative_collection_metadata_vs_ddls.js | 28 +++++++ ...ck_unsplittable_collection_coordinator.cpp | 73 +++++++++++++++---- 3 files changed, 97 insertions(+), 18 deletions(-) diff --git a/jstests/noPassthrough/ddl/untrack_unsharded_collection_with_orphaned_data.js b/jstests/noPassthrough/ddl/untrack_unsharded_collection_with_orphaned_data.js index 56cc4742b2a..3f80b70b2fc 100644 --- a/jstests/noPassthrough/ddl/untrack_unsharded_collection_with_orphaned_data.js +++ b/jstests/noPassthrough/ddl/untrack_unsharded_collection_with_orphaned_data.js @@ -5,6 +5,7 @@ * requires_fcv_80, * ] */ +import {FeatureFlagUtil} from "jstests/libs/feature_flag_util.js"; import {ShardingTest} from "jstests/libs/shardingtest.js"; // The test injects a metadata inconsistency. @@ -15,7 +16,7 @@ const shard0Name = st.shard0.shardName; const shard1Name = st.shard1.shardName; let lastUUID = null; -const kDbName = "db"; +const kDbName = jsTestName(); const kCollName = "coll"; const kNss = kDbName + "." + kCollName; @@ -25,6 +26,15 @@ function verifyShardingCatalogStateAfterUntracking(st, primaryShard, ns, uuid) { // Make sure there is no entry on config.chunks. assert.eq(0, st.s.getCollection("config.chunks").countDocuments({uuid: uuid})); + if (FeatureFlagUtil.isPresentAndEnabled(st.s, "AuthoritativeShardsDDL")) { + [st.shard0, st.shard1].forEach((shard) => { + const configDb = shard.getDB("config"); + assert.eq(null, configDb.getCollection("shard.catalog.collections").findOne({_id: ns})); + assert.eq(0, configDb.getCollection("shard.catalog.chunks").countDocuments({uuid: uuid})); + }); + return; + } + // Make sure that the primary shard refreshed its filtering metadata upon completing the // command, so that there is no document on the related collections. const chunksCollName = "cache.chunks." + ns; @@ -54,7 +64,7 @@ jsTest.log("Untrack a collection on a new primary shard works but non-empty orph // Inject an orphaned document for the collection in the primary shard; // The upcoming untrackCollection command is expected to leave it untouched (since the // collection is not empty). - st.rs1.getPrimary().getCollection(kNss).insert({x: 1}); + assert.commandWorked(st.rs1.getPrimary().getCollection(kNss).insert({x: 1})); // Untrack the collection; the operation is only expected to succeed when its data are located // on the primary shard. diff --git a/jstests/noPassthrough/global_catalog/authoritative_collection_metadata_vs_ddls.js b/jstests/noPassthrough/global_catalog/authoritative_collection_metadata_vs_ddls.js index c8dd048070c..1e3d591c194 100644 --- a/jstests/noPassthrough/global_catalog/authoritative_collection_metadata_vs_ddls.js +++ b/jstests/noPassthrough/global_catalog/authoritative_collection_metadata_vs_ddls.js @@ -393,6 +393,34 @@ describe("Authoritative collection metadata vs DDLs", function () { }); }); + describe("untrackUnshardedCollection", function () { + it("cleans up shard catalog and preserves data for a tracked unsplittable collection", function () { + const db = setupDb("untrack"); + const ns = `${db.getName()}.coll`; + + assert.commandWorked(db.createCollection("coll")); + assert.commandWorked(db.adminCommand({moveCollection: ns, toShard: st.shard1.shardName})); + assert.commandWorked(db.adminCommand({moveCollection: ns, toShard: st.shard0.shardName})); + assert.commandWorked(db.coll.insert([{x: 1}, {x: 2}])); + + const globalMeta = getGlobalCatalogCollMetadata(ns); + assert.neq(null, globalMeta, `${ns}: expected in global catalog before untrack`); + const uuid = globalMeta.uuid; + + assert.commandWorked(db.adminCommand({untrackUnshardedCollection: ns})); + + assert.eq(null, getGlobalCatalogCollMetadata(ns), `${ns}: still in global catalog after untrack`); + assert.eq(0, getAllGlobalCatalogChunks(uuid).length, `${ns}: chunks still in global catalog after untrack`); + assert.eq(2, db.coll.countDocuments({}), `${ns}: user data should remain after untrack`); + + st.awaitReplicationOnShards(); + forEachNodeOnAllShards((node) => { + assertShardCatalogAbsentOnNode(node, ns, uuid); + assertInMemoryMetadataNotSharded(node, ns); + }); + }); + }); + describe("dropDatabase", function () { it("cleans up shard catalog for all tracked collections in the database", function () { const db = setupDb("dropdb"); diff --git a/src/mongo/db/global_catalog/ddl/untrack_unsplittable_collection_coordinator.cpp b/src/mongo/db/global_catalog/ddl/untrack_unsplittable_collection_coordinator.cpp index 27e1891cb97..295a79510f7 100644 --- a/src/mongo/db/global_catalog/ddl/untrack_unsplittable_collection_coordinator.cpp +++ b/src/mongo/db/global_catalog/ddl/untrack_unsplittable_collection_coordinator.cpp @@ -100,11 +100,21 @@ void UntrackUnsplittableCollectionCoordinator::_enterCriticalSection( std::shared_ptr executor, const CancellationToken& token) { auto service = ShardingRecoveryService::get(opCtx); + const bool isAuthoritative = _doc.getAuthoritativeMetadataAccessLevel() >= + AuthoritativeMetadataAccessLevelEnum::kWritesAllowed; + const bool clearFilteringMetadata = !isAuthoritative; + + // The critical-section document controls what secondaries do when they observe the release. + // With shard-authoritative collection metadata, the commit phase removes the shard catalog + // entries and invalidates filtering metadata, so secondaries should not also clear collection + // metadata from critical-section cleanup. service->acquireRecoverableCriticalSectionBlockWrites( opCtx, nss(), _critSecReason, - ShardingCatalogClient::writeConcernLocalHavingUpstreamWaiter()); + ShardingCatalogClient::writeConcernLocalHavingUpstreamWaiter(), + false /* clearDbMetadata: untrack only changes collection metadata */, + clearFilteringMetadata); service->promoteRecoverableCriticalSectionToBlockAlsoReads( opCtx, nss(), @@ -120,6 +130,7 @@ void UntrackUnsplittableCollectionCoordinator::_commitUntrackCollection( std::shared_ptr executor, const CancellationToken& token) { tassert(8631102, "There must be a collection stored in the document", _doc.getOptCollType()); + const auto& coll = _doc.getOptCollType().get(); if (!_firstExecution) { AllShardsAndConfigCausalityBarrier barrier{**executor, token}; @@ -132,12 +143,29 @@ void UntrackUnsplittableCollectionCoordinator::_commitUntrackCollection( opCtx, Grid::get(opCtx)->shardRegistry()->getConfigShard(), Grid::get(opCtx)->catalogClient(), - _doc.getOptCollType().get(), + coll, defaultMajorityWriteConcernDoNotUse(), session, **executor); } + const bool isAuthoritative = _doc.getAuthoritativeMetadataAccessLevel() >= + AuthoritativeMetadataAccessLevelEnum::kWritesAllowed; + + // The global catalog no longer tracks the collection, so shards must forget the corresponding + // local shard-catalog entries before the critical section is released. + if (isAuthoritative) { + const auto session = getNewSession(opCtx); + sharding_ddl_util::commitDropCollectionMetadataToShardCatalog( + opCtx, + nss(), + coll.getUuid(), + Grid::get(opCtx)->shardRegistry()->getAllShardIds(opCtx), + session, + executor, + token); + } + // Checkpoint the configTime to ensure that, in the case of a stepdown, the new primary will // start-up from a configTime that is inclusive of the metadata deletions that were committed // during the critical section. @@ -152,7 +180,9 @@ void UntrackUnsplittableCollectionCoordinator::_commitUntrackCollection( participants.end()); { const auto session = getNewSession(opCtx); - const auto uuid = sharding_ddl_util::getCollectionUUID(opCtx, nss()); + // In authoritative mode, the shard-catalog commit above already handled metadata + // invalidation, so this participant command only needs to clean up data from local + // collections without clearing the collection metadata again. sharding_ddl_util::sendDropCollectionParticipantCommandToShards( opCtx, nss(), @@ -162,8 +192,9 @@ void UntrackUnsplittableCollectionCoordinator::_commitUntrackCollection( session, true /* fromMigrate */, false /* dropSystemCollections */, - uuid, - true /*requireCollectionEmpty*/); + coll.getUuid(), + true /*requireCollectionEmpty*/, + !isAuthoritative /* forceLegacyRefresh */); } } @@ -171,23 +202,33 @@ void UntrackUnsplittableCollectionCoordinator::_exitCriticalSection( OperationContext* opCtx, std::shared_ptr executor, const CancellationToken& token) { - // Force a refresh of the filtering metadata to clean up the data structure held by the - // CollectionShardingRuntime (Note also that this code is indirectly used to notify to secondary - // nodes to clear their filtering information). - FilteringMetadataCache::get(opCtx)->forceCollectionPlacementRefresh(opCtx, nss()); - FilteringMetadataCache::get(opCtx)->waitForCollectionFlush(opCtx, nss()); + const bool isAuthoritative = _doc.getAuthoritativeMetadataAccessLevel() >= + AuthoritativeMetadataAccessLevelEnum::kWritesAllowed; - // Ensures the refresh of the catalog cache will be waited majority at the end of the - // command - repl::ReplClientInfo::forClient(opCtx->getClient()).setLastOpToSystemLastOpTime(opCtx); + if (!isAuthoritative) { + // Legacy readers rely on the filtering metadata refresh to clear the cached state on the + // primary and to replicate the invalidate to secondaries. + FilteringMetadataCache::get(opCtx)->forceCollectionPlacementRefresh(opCtx, nss()); + FilteringMetadataCache::get(opCtx)->waitForCollectionFlush(opCtx, nss()); + repl::ReplClientInfo::forClient(opCtx->getClient()).setLastOpToSystemLastOpTime(opCtx); + } - auto service = ShardingRecoveryService::get(opCtx); - service->releaseRecoverableCriticalSection( + std::unique_ptr actionPtr; + if (isAuthoritative) { + // The commit phase already removed the durable shard catalog entries and invalidated + // in-memory metadata, so releasing the critical section must not perform a second clear + // that could race with the authoritative commit semantics. + actionPtr = std::make_unique(); + } else { + actionPtr = std::make_unique(); + } + + ShardingRecoveryService::get(opCtx)->releaseRecoverableCriticalSection( opCtx, nss(), _critSecReason, ShardingCatalogClient::writeConcernLocalHavingUpstreamWaiter(), - ShardingRecoveryService::FilteringMetadataClearer()); + *actionPtr); ShardingLogging::get(opCtx)->logChange(opCtx, "untrackCollection.end", nss()); }