From 5a6bcdf77dc1769e2542cb16957fb69073306d03 Mon Sep 17 00:00:00 2001 From: Sergi Mateo Bellido Date: Tue, 14 Sep 2021 13:07:52 +0000 Subject: SERVER-58466 Minor optimization on the CatalogCache --- src/mongo/db/s/balancer/balance_stats_test.cpp | 3 +- .../resharding_data_replication_test.cpp | 3 +- .../resharding_oplog_batch_applier_test.cpp | 3 +- .../resharding_oplog_crud_application_test.cpp | 3 +- .../resharding_recipient_service_test.cpp | 3 +- src/mongo/db/s/sharding_write_router_bm.cpp | 3 +- src/mongo/s/catalog_cache.cpp | 59 ++++++++++++++-------- src/mongo/s/chunk_manager.h | 18 ++----- src/mongo/s/chunk_manager_refresh_bm.cpp | 3 +- src/mongo/s/sharding_test_fixture_common.cpp | 3 +- 10 files changed, 58 insertions(+), 43 deletions(-) diff --git a/src/mongo/db/s/balancer/balance_stats_test.cpp b/src/mongo/db/s/balancer/balance_stats_test.cpp index a4c2f2b00bf..9381e0a2da6 100644 --- a/src/mongo/db/s/balancer/balance_stats_test.cpp +++ b/src/mongo/db/s/balancer/balance_stats_test.cpp @@ -67,7 +67,8 @@ public: return ChunkManager(_shardPrimary, _dbVersion, - RoutingTableHistoryValueHandle(std::move(routingTableHistory)), + RoutingTableHistoryValueHandle(std::make_shared( + std::move(routingTableHistory))), boost::none); } diff --git a/src/mongo/db/s/resharding/resharding_data_replication_test.cpp b/src/mongo/db/s/resharding/resharding_data_replication_test.cpp index b33c3487be9..9be48c3c59d 100644 --- a/src/mongo/db/s/resharding/resharding_data_replication_test.cpp +++ b/src/mongo/db/s/resharding/resharding_data_replication_test.cpp @@ -102,7 +102,8 @@ private: RoutingTableHistoryValueHandle makeStandaloneRoutingTableHistory(RoutingTableHistory rt) { const auto version = rt.getVersion(); return RoutingTableHistoryValueHandle( - std::move(rt), ComparableChunkVersion::makeComparableChunkVersion(version)); + std::make_shared(std::move(rt)), + ComparableChunkVersion::makeComparableChunkVersion(version)); } const StringData _currentShardKey = "sk"; diff --git a/src/mongo/db/s/resharding/resharding_oplog_batch_applier_test.cpp b/src/mongo/db/s/resharding/resharding_oplog_batch_applier_test.cpp index c72b4a2122a..2e741189743 100644 --- a/src/mongo/db/s/resharding/resharding_oplog_batch_applier_test.cpp +++ b/src/mongo/db/s/resharding/resharding_oplog_batch_applier_test.cpp @@ -324,7 +324,8 @@ private: RoutingTableHistoryValueHandle makeStandaloneRoutingTableHistory(RoutingTableHistory rt) { const auto version = rt.getVersion(); return RoutingTableHistoryValueHandle( - std::move(rt), ComparableChunkVersion::makeComparableChunkVersion(version)); + std::make_shared(std::move(rt)), + ComparableChunkVersion::makeComparableChunkVersion(version)); } const StringData _currentShardKey = "sk"; diff --git a/src/mongo/db/s/resharding/resharding_oplog_crud_application_test.cpp b/src/mongo/db/s/resharding/resharding_oplog_crud_application_test.cpp index 4be2c87a008..ad8df37a2b8 100644 --- a/src/mongo/db/s/resharding/resharding_oplog_crud_application_test.cpp +++ b/src/mongo/db/s/resharding/resharding_oplog_crud_application_test.cpp @@ -311,7 +311,8 @@ private: RoutingTableHistoryValueHandle makeStandaloneRoutingTableHistory(RoutingTableHistory rt) { const auto version = rt.getVersion(); return RoutingTableHistoryValueHandle( - std::move(rt), ComparableChunkVersion::makeComparableChunkVersion(version)); + std::make_shared(std::move(rt)), + ComparableChunkVersion::makeComparableChunkVersion(version)); } const StringData _currentShardKey = "sk"; diff --git a/src/mongo/db/s/resharding/resharding_recipient_service_test.cpp b/src/mongo/db/s/resharding/resharding_recipient_service_test.cpp index 5ec1423036c..5d69b790321 100644 --- a/src/mongo/db/s/resharding/resharding_recipient_service_test.cpp +++ b/src/mongo/db/s/resharding/resharding_recipient_service_test.cpp @@ -139,7 +139,8 @@ private: RoutingTableHistoryValueHandle _makeStandaloneRoutingTableHistory(RoutingTableHistory rt) { const auto version = rt.getVersion(); return RoutingTableHistoryValueHandle( - std::move(rt), ComparableChunkVersion::makeComparableChunkVersion(version)); + std::make_shared(std::move(rt)), + ComparableChunkVersion::makeComparableChunkVersion(version)); } const StringData _currentShardKey = "oldKey"; diff --git a/src/mongo/db/s/sharding_write_router_bm.cpp b/src/mongo/db/s/sharding_write_router_bm.cpp index 8d354b1198c..d090a015946 100644 --- a/src/mongo/db/s/sharding_write_router_bm.cpp +++ b/src/mongo/db/s/sharding_write_router_bm.cpp @@ -81,7 +81,8 @@ ChunkRange getRangeForChunk(int i, int nChunks) { RoutingTableHistoryValueHandle makeStandaloneRoutingTableHistory(RoutingTableHistory rt) { const auto version = rt.getVersion(); return RoutingTableHistoryValueHandle( - std::move(rt), ComparableChunkVersion::makeComparableChunkVersion(version)); + std::make_shared(std::move(rt)), + ComparableChunkVersion::makeComparableChunkVersion(version)); } std::pair, mongo::ChunkManager> createChunks( diff --git a/src/mongo/s/catalog_cache.cpp b/src/mongo/s/catalog_cache.cpp index 933ecbd0df1..9e3eea4a5a4 100644 --- a/src/mongo/s/catalog_cache.cpp +++ b/src/mongo/s/catalog_cache.cpp @@ -654,6 +654,43 @@ CatalogCache::CollectionCache::LookupResult CatalogCache::CollectionCache::_look auto collectionAndChunks = _catalogCacheLoader.getChunksSince(nss, lookupVersion).get(); + // If a refresh doesn't find new information -> re-use the existing RoutingTableHistory + if (isIncremental && collectionAndChunks.changedChunks.size() == 1 && + collectionAndChunks.changedChunks[0].getVersion() == + existingHistory->optRt->getVersion()) { + + invariant( + collectionAndChunks.allowMigrations == existingHistory->optRt->allowMigrations(), + str::stream() << "allowMigrations field of " << nss + << " collection changed without changing the collection version " + << existingHistory->optRt->getVersion().toString() + << ". Old value: " << existingHistory->optRt->allowMigrations() + << ", new value: " << collectionAndChunks.allowMigrations); + + + const auto& oldReshardingFields = existingHistory->optRt->getReshardingFields(); + const auto& newReshardingFields = collectionAndChunks.reshardingFields; + + invariant( + [&] { + if (oldReshardingFields && newReshardingFields) + return oldReshardingFields->toBSON().woCompare( + newReshardingFields->toBSON()) == 0; + else + return !oldReshardingFields && !newReshardingFields; + }(), + str::stream() << "reshardingFields field of " << nss + << " collection changed without changing the collection version " + << existingHistory->optRt->getVersion().toString() + << ". Old value: " << oldReshardingFields->toBSON() + << ", new value: " << newReshardingFields->toBSON()); + + // Despite we didn't find new info, we must update the time of this entry on the cache + newComparableVersion.setChunkVersion(collectionAndChunks.changedChunks[0].getVersion()); + return LookupResult(OptionalRoutingTableHistory(existingHistory->optRt), + std::move(newComparableVersion)); + } + const auto maxChunkSize = [&]() -> boost::optional { if (!collectionAndChunks.allowAutoSplit) { // maxChunkSize = 0 is an invalid chunkSize so we use it to detect noAutoSplit @@ -715,27 +752,8 @@ CatalogCache::CollectionCache::LookupResult CatalogCache::CollectionCache::_look } const ChunkVersion newVersion = newRoutingHistory.getVersion(); - newComparableVersion.setChunkVersion(newVersion); - if (isIncremental && existingHistory->optRt->getVersion() == newVersion) { - invariant(newRoutingHistory.sameAllowMigrations(*existingHistory->optRt), - str::stream() - << "allowMigrations field of " << nss - << " collection changed without changing the collection version " - << newVersion.toString() - << ". Old value: " << existingHistory->optRt->allowMigrations() - << ", new value: " << newRoutingHistory.allowMigrations()); - - invariant(newRoutingHistory.sameReshardingFields(*existingHistory->optRt), - str::stream() - << "reshardingFields field of " << nss - << " collection changed without changing the collection version " - << newVersion.toString() << ". Old value: " - << existingHistory->optRt->getReshardingFields()->toBSON() - << ", new value: " << newRoutingHistory.getReshardingFields()->toBSON()); - } - LOGV2_FOR_CATALOG_REFRESH(4619901, isIncremental || newComparableVersion != previousVersion ? 0 : 1, "Refreshed cached collection", @@ -746,7 +764,8 @@ CatalogCache::CollectionCache::LookupResult CatalogCache::CollectionCache::_look "duration"_attr = Milliseconds(t.millis())); _updateRefreshesStats(isIncremental, false); - return LookupResult(OptionalRoutingTableHistory(std::move(newRoutingHistory)), + return LookupResult(OptionalRoutingTableHistory(std::make_shared( + std::move(newRoutingHistory))), std::move(newComparableVersion)); } catch (const DBException& ex) { _stats.countFailedRefreshes.addAndFetch(1); diff --git a/src/mongo/s/chunk_manager.h b/src/mongo/s/chunk_manager.h index 9d48ccfb9eb..dca4d973ace 100644 --- a/src/mongo/s/chunk_manager.h +++ b/src/mongo/s/chunk_manager.h @@ -290,18 +290,6 @@ public: return _uuid && *_uuid == uuid; } - bool sameAllowMigrations(const RoutingTableHistory& other) const { - return _allowMigrations == other._allowMigrations; - } - - bool sameReshardingFields(const RoutingTableHistory& other) const { - if (_reshardingFields && other._reshardingFields) { - return _reshardingFields->toBSON().woCompare(other._reshardingFields->toBSON()) == 0; - } else { - return !_reshardingFields && !other._reshardingFields; - } - } - boost::optional getUUID() const { return _uuid; } @@ -484,10 +472,10 @@ struct OptionalRoutingTableHistory { OptionalRoutingTableHistory() = default; // SHARDED collection constructor - OptionalRoutingTableHistory(RoutingTableHistory&& rt) : optRt(std::move(rt)) {} + OptionalRoutingTableHistory(std::shared_ptr rt) : optRt(std::move(rt)) {} - // If boost::none, the collection is UNSHARDED, otherwise it is SHARDED - boost::optional optRt; + // If nullptr, the collection is UNSHARDED, otherwise it is SHARDED + std::shared_ptr optRt; }; using RoutingTableHistoryCache = diff --git a/src/mongo/s/chunk_manager_refresh_bm.cpp b/src/mongo/s/chunk_manager_refresh_bm.cpp index 611b7826c7c..4aec67285e1 100644 --- a/src/mongo/s/chunk_manager_refresh_bm.cpp +++ b/src/mongo/s/chunk_manager_refresh_bm.cpp @@ -46,7 +46,8 @@ const NamespaceString kNss("test", "foo"); RoutingTableHistoryValueHandle makeStandaloneRoutingTableHistory(RoutingTableHistory rt) { const auto version = rt.getVersion(); return RoutingTableHistoryValueHandle( - std::move(rt), ComparableChunkVersion::makeComparableChunkVersion(version)); + std::make_shared(std::move(rt)), + ComparableChunkVersion::makeComparableChunkVersion(version)); } ChunkRange getRangeForChunk(int i, int nChunks) { diff --git a/src/mongo/s/sharding_test_fixture_common.cpp b/src/mongo/s/sharding_test_fixture_common.cpp index 12ef4fd5006..f136cadc42c 100644 --- a/src/mongo/s/sharding_test_fixture_common.cpp +++ b/src/mongo/s/sharding_test_fixture_common.cpp @@ -68,7 +68,8 @@ RoutingTableHistoryValueHandle ShardingTestFixtureCommon::makeStandaloneRoutingT RoutingTableHistory rt) { const auto version = rt.getVersion(); return RoutingTableHistoryValueHandle( - std::move(rt), ComparableChunkVersion::makeComparableChunkVersion(version)); + std::make_shared(std::move(rt)), + ComparableChunkVersion::makeComparableChunkVersion(version)); } void ShardingTestFixtureCommon::onCommand(NetworkTestEnv::OnCommandFunction func) { -- cgit v1.2.1