From c41763970932ee2534ad932d557d322ce94f692f Mon Sep 17 00:00:00 2001 From: Antonio Fuschetto Date: Wed, 2 Mar 2022 10:06:06 +0000 Subject: SERVER-63742 Default topology time in shard can lead to infinite refresh in shard registry --- src/mongo/db/pipeline/variables.cpp | 2 +- src/mongo/db/read_concern_mongod.cpp | 2 +- src/mongo/db/s/vector_clock_config_server_test.cpp | 4 +- src/mongo/db/s/vector_clock_shard_server_test.cpp | 32 ++++++++------ src/mongo/db/vector_clock.cpp | 4 +- src/mongo/db/vector_clock.h | 19 ++++++++- src/mongo/db/vector_clock_document.idl | 2 + src/mongo/db/vector_clock_mongod.cpp | 9 ++-- src/mongo/db/vector_clock_mongod_test.cpp | 24 +++++------ src/mongo/db/vector_clock_test.cpp | 4 +- src/mongo/s/balancer_configuration_test.cpp | 5 ++- .../s/catalog/sharding_catalog_client_test.cpp | 49 ++++++++++++++++------ src/mongo/s/client/shard_remote_test.cpp | 5 ++- src/mongo/s/commands/strategy.cpp | 4 +- src/mongo/s/sharding_router_test_fixture.cpp | 9 +++- 15 files changed, 118 insertions(+), 56 deletions(-) diff --git a/src/mongo/db/pipeline/variables.cpp b/src/mongo/db/pipeline/variables.cpp index 0adcf0505ff..26793962efd 100644 --- a/src/mongo/db/pipeline/variables.cpp +++ b/src/mongo/db/pipeline/variables.cpp @@ -281,7 +281,7 @@ LegacyRuntimeConstants Variables::generateRuntimeConstants(OperationContext* opC if (opCtx->getClient()) { if (const auto vectorClock = VectorClock::get(opCtx)) { const auto now = vectorClock->getTime(); - if (now.clusterTime() != LogicalTime::kUninitialized) { + if (VectorClock::isValidComponentTime(now.clusterTime())) { return {Date_t::now(), now.clusterTime().asTimestamp()}; } } diff --git a/src/mongo/db/read_concern_mongod.cpp b/src/mongo/db/read_concern_mongod.cpp index ea339471f8e..48637beefd5 100644 --- a/src/mongo/db/read_concern_mongod.cpp +++ b/src/mongo/db/read_concern_mongod.cpp @@ -352,7 +352,7 @@ Status waitForReadConcernImpl(OperationContext* opCtx, const auto currentTime = VectorClock::get(opCtx)->getTime(); const auto clusterTime = currentTime.clusterTime(); - if (clusterTime == LogicalTime::kUninitialized) { + if (!VectorClock::isValidComponentTime(clusterTime)) { // currentTime should only be uninitialized if we are in startup recovery or initial // sync. invariant(memberState.startup() || memberState.startup2()); diff --git a/src/mongo/db/s/vector_clock_config_server_test.cpp b/src/mongo/db/s/vector_clock_config_server_test.cpp index f9246daa809..14694f697d0 100644 --- a/src/mongo/db/s/vector_clock_config_server_test.cpp +++ b/src/mongo/db/s/vector_clock_config_server_test.cpp @@ -128,7 +128,7 @@ TEST_F(VectorClockConfigServerTest, TickToConfigTime) { auto vc = VectorClockMutable::get(sc); const auto t0 = vc->getTime(); - ASSERT_EQ(LogicalTime(), t0.configTime()); + ASSERT_EQ(VectorClock::kInitialComponentTime, t0.configTime()); vc->tickConfigTimeTo(LogicalTime(Timestamp(1, 1))); const auto t1 = vc->getTime(); @@ -154,7 +154,7 @@ TEST_F(VectorClockConfigServerTest, TickToTopologyTime) { auto vc = VectorClockMutable::get(sc); const auto t0 = vc->getTime(); - ASSERT_EQ(LogicalTime(), t0.topologyTime()); + ASSERT_EQ(VectorClock::kInitialComponentTime, t0.topologyTime()); vc->tickTopologyTimeTo(LogicalTime(Timestamp(1, 1))); const auto t1 = vc->getTime(); diff --git a/src/mongo/db/s/vector_clock_shard_server_test.cpp b/src/mongo/db/s/vector_clock_shard_server_test.cpp index 166d0802a55..0674419fc34 100644 --- a/src/mongo/db/s/vector_clock_shard_server_test.cpp +++ b/src/mongo/db/s/vector_clock_shard_server_test.cpp @@ -161,9 +161,9 @@ TEST_F(VectorClockShardServerTest, GossipOutInternal) { ASSERT_TRUE(obj.hasField("$clusterTime")); ASSERT_EQ(obj["$clusterTime"].Obj()["clusterTime"].timestamp(), clusterTime.asTimestamp()); ASSERT_TRUE(obj.hasField("$configTime")); - ASSERT_EQ(obj["$configTime"].timestamp(), Timestamp(0, 0)); + ASSERT_EQ(obj["$configTime"].timestamp(), VectorClock::kInitialComponentTime.asTimestamp()); ASSERT_TRUE(obj.hasField("$topologyTime")); - ASSERT_EQ(obj["$topologyTime"].timestamp(), Timestamp(0, 0)); + ASSERT_EQ(obj["$topologyTime"].timestamp(), VectorClock::kInitialComponentTime.asTimestamp()); } TEST_F(VectorClockShardServerTest, GossipOutExternal) { @@ -254,8 +254,8 @@ TEST_F(VectorClockShardServerTest, GossipInExternal) { // $configTime or $topologyTime. auto afterTime = vc->getTime(); ASSERT_EQ(afterTime.clusterTime().asTimestamp(), Timestamp(2, 2)); - ASSERT_EQ(afterTime.configTime().asTimestamp(), Timestamp(0, 0)); - ASSERT_EQ(afterTime.topologyTime().asTimestamp(), Timestamp(0, 0)); + ASSERT_EQ(afterTime.configTime(), VectorClock::kInitialComponentTime); + ASSERT_EQ(afterTime.topologyTime(), VectorClock::kInitialComponentTime); vc->gossipIn(nullptr, BSON("$clusterTime" @@ -265,8 +265,8 @@ TEST_F(VectorClockShardServerTest, GossipInExternal) { auto afterTime2 = vc->getTime(); ASSERT_EQ(afterTime2.clusterTime().asTimestamp(), Timestamp(2, 2)); - ASSERT_EQ(afterTime2.configTime().asTimestamp(), Timestamp(0, 0)); - ASSERT_EQ(afterTime2.topologyTime().asTimestamp(), Timestamp(0, 0)); + ASSERT_EQ(afterTime2.configTime(), VectorClock::kInitialComponentTime); + ASSERT_EQ(afterTime2.topologyTime(), VectorClock::kInitialComponentTime); vc->gossipIn(nullptr, BSON("$clusterTime" @@ -276,8 +276,8 @@ TEST_F(VectorClockShardServerTest, GossipInExternal) { auto afterTime3 = vc->getTime(); ASSERT_EQ(afterTime3.clusterTime().asTimestamp(), Timestamp(3, 3)); - ASSERT_EQ(afterTime3.configTime().asTimestamp(), Timestamp(0, 0)); - ASSERT_EQ(afterTime3.topologyTime().asTimestamp(), Timestamp(0, 0)); + ASSERT_EQ(afterTime3.configTime(), VectorClock::kInitialComponentTime); + ASSERT_EQ(afterTime3.topologyTime(), VectorClock::kInitialComponentTime); } class VectorClockPersistenceTest : public ShardServerTestFixture { @@ -329,8 +329,8 @@ TEST_F(VectorClockPersistenceTest, PrimaryRecoverWithoutExistingVectorClockDocum vc->recoverDirect(opCtx); auto time = vc->getTime(); - ASSERT_EQ(Timestamp(0), time.configTime().asTimestamp()); - ASSERT_EQ(Timestamp(0), time.topologyTime().asTimestamp()); + ASSERT_EQ(VectorClock::kInitialComponentTime, time.configTime()); + ASSERT_EQ(VectorClock::kInitialComponentTime, time.topologyTime()); } TEST_F(VectorClockPersistenceTest, PrimaryRecoverWithExistingVectorClockDocument) { @@ -341,7 +341,10 @@ TEST_F(VectorClockPersistenceTest, PrimaryRecoverWithExistingVectorClockDocument // Check that no vectorClockState document is present PersistentTaskStore store(NamespaceString::kVectorClockNamespace); ASSERT_EQ(store.count(opCtx, kVectorClockQuery), 0); - store.add(opCtx, VectorClockDocument(Timestamp(100), Timestamp(50))); + VectorClockDocument vcd; + vcd.setConfigTime(Timestamp(100)); + vcd.setTopologyTime(Timestamp(50)); + store.add(opCtx, vcd); vc->recoverDirect(opCtx); @@ -365,8 +368,11 @@ TEST_F(VectorClockPersistenceTest, PrimaryRecoverWithIllegalVectorClockDocument) << "IllegalKey" << "IllegalValue")); - const int kParseErrorCode = 40414; - ASSERT_THROWS_CODE(vc->recoverDirect(opCtx), DBException, ErrorCodes::Error(kParseErrorCode)); + vc->recoverDirect(opCtx); + + auto time = vc->getTime(); + ASSERT_EQ(VectorClock::kInitialComponentTime, time.configTime()); + ASSERT_EQ(VectorClock::kInitialComponentTime, time.topologyTime()); } } // namespace diff --git a/src/mongo/db/vector_clock.cpp b/src/mongo/db/vector_clock.cpp index 6571f3b1efc..076d025889a 100644 --- a/src/mongo/db/vector_clock.cpp +++ b/src/mongo/db/vector_clock.cpp @@ -45,6 +45,8 @@ const auto vectorClockDecoration = ServiceContext::declareDecoration lock(_mutex); auto it = _vectorTime.begin(); for (; it != _vectorTime.end(); ++it) { - *it = LogicalTime(); + *it = VectorClock::kInitialComponentTime; } _isEnabled = true; } diff --git a/src/mongo/db/vector_clock.h b/src/mongo/db/vector_clock.h index ba2af47af80..eb40b36f499 100644 --- a/src/mongo/db/vector_clock.h +++ b/src/mongo/db/vector_clock.h @@ -87,7 +87,7 @@ public: class VectorTime { public: explicit VectorTime(LogicalTimeArray time) : _time(std::move(time)) {} - VectorTime() = default; + VectorTime() = delete; LogicalTime clusterTime() const& { return _time[Component::ClusterTime]; @@ -115,6 +115,20 @@ public: LogicalTimeArray _time; }; + // There is a special logic in the storage engine which fixes up Timestamp(0, 0) to the latest + // available time on the node. Because of this, we should never gossip or have a VectorClock + // initialised with a value of Timestamp(0, 0), because that would cause the checkpointed value + // to move forward in time. + static const LogicalTime kInitialComponentTime; + + /** + * Returns true if the passed LogicalTime is set to a value higher than kInitialComponentTime, + * false otherwise. + */ + static bool isValidComponentTime(const LogicalTime& time) { + return time > kInitialComponentTime; + } + static constexpr char kClusterTimeFieldName[] = "$clusterTime"; static constexpr char kConfigTimeFieldName[] = "$configTime"; static constexpr char kTopologyTimeFieldName[] = "$topologyTime"; @@ -294,7 +308,8 @@ protected: bool _isEnabled{true}; - LogicalTimeArray _vectorTime; + LogicalTimeArray _vectorTime = { + kInitialComponentTime, kInitialComponentTime, kInitialComponentTime}; private: class PlainComponentFormat; diff --git a/src/mongo/db/vector_clock_document.idl b/src/mongo/db/vector_clock_document.idl index d35263911e5..1db0f4c35e4 100644 --- a/src/mongo/db/vector_clock_document.idl +++ b/src/mongo/db/vector_clock_document.idl @@ -46,6 +46,8 @@ structs: configTime: type: timestamp description: "Persists the configTime component of the vector clock" + default: Timestamp(0, 1) topologyTime: type: timestamp description: "Persists the topologyTime component of the vector clock" + default: Timestamp(0, 1) diff --git a/src/mongo/db/vector_clock_mongod.cpp b/src/mongo/db/vector_clock_mongod.cpp index f5d7513a543..93db5aa123c 100644 --- a/src/mongo/db/vector_clock_mongod.cpp +++ b/src/mongo/db/vector_clock_mongod.cpp @@ -267,7 +267,7 @@ VectorClock::VectorTime VectorClockMongoD::recoverDirect(OperationContext* opCtx return true; }); - const auto newDurableTime = VectorTime({LogicalTime(Timestamp(0)), + const auto newDurableTime = VectorTime({VectorClock::kInitialComponentTime, LogicalTime(durableVectorClock.getConfigTime()), LogicalTime(durableVectorClock.getTopologyTime())}); @@ -364,14 +364,17 @@ Future VectorClockMongoD::_doWhileQueueNotEmptyOrError(ServiceContext* ser } auto vectorTime = getTime(); - const VectorClockDocument vcd(vectorTime.configTime().asTimestamp(), - vectorTime.topologyTime().asTimestamp()); + + VectorClockDocument vcd; + vcd.setConfigTime(vectorTime.configTime().asTimestamp()); + vcd.setTopologyTime(vectorTime.topologyTime().asTimestamp()); PersistentTaskStore store(NamespaceString::kVectorClockNamespace); store.upsert(opCtx, BSON(VectorClockDocument::k_idFieldName << vcd.get_id()), vcd.toBSON(), WriteConcerns::kMajorityWriteConcernNoTimeout); + return vectorTime; }) .getAsync([this, promise = std::move(p)](StatusWith swResult) mutable { diff --git a/src/mongo/db/vector_clock_mongod_test.cpp b/src/mongo/db/vector_clock_mongod_test.cpp index 9ea7801f92e..51166840c89 100644 --- a/src/mongo/db/vector_clock_mongod_test.cpp +++ b/src/mongo/db/vector_clock_mongod_test.cpp @@ -216,8 +216,8 @@ TEST_F(VectorClockMongoDTest, GossipInInternal) { // $configTime or $topologyTime. auto afterTime = vc->getTime(); ASSERT_EQ(afterTime.clusterTime().asTimestamp(), Timestamp(2, 2)); - ASSERT_EQ(afterTime.configTime().asTimestamp(), Timestamp(0, 0)); - ASSERT_EQ(afterTime.topologyTime().asTimestamp(), Timestamp(0, 0)); + ASSERT_EQ(afterTime.configTime(), VectorClock::kInitialComponentTime); + ASSERT_EQ(afterTime.topologyTime(), VectorClock::kInitialComponentTime); vc->gossipIn(nullptr, BSON("$clusterTime" @@ -228,8 +228,8 @@ TEST_F(VectorClockMongoDTest, GossipInInternal) { auto afterTime2 = vc->getTime(); ASSERT_EQ(afterTime2.clusterTime().asTimestamp(), Timestamp(2, 2)); - ASSERT_EQ(afterTime2.configTime().asTimestamp(), Timestamp(0, 0)); - ASSERT_EQ(afterTime2.topologyTime().asTimestamp(), Timestamp(0, 0)); + ASSERT_EQ(afterTime2.configTime(), VectorClock::kInitialComponentTime); + ASSERT_EQ(afterTime2.topologyTime(), VectorClock::kInitialComponentTime); vc->gossipIn(nullptr, BSON("$clusterTime" @@ -240,8 +240,8 @@ TEST_F(VectorClockMongoDTest, GossipInInternal) { auto afterTime3 = vc->getTime(); ASSERT_EQ(afterTime3.clusterTime().asTimestamp(), Timestamp(3, 3)); - ASSERT_EQ(afterTime3.configTime().asTimestamp(), Timestamp(0, 0)); - ASSERT_EQ(afterTime3.topologyTime().asTimestamp(), Timestamp(0, 0)); + ASSERT_EQ(afterTime3.configTime(), VectorClock::kInitialComponentTime); + ASSERT_EQ(afterTime3.topologyTime(), VectorClock::kInitialComponentTime); } TEST_F(VectorClockMongoDTest, GossipInExternal) { @@ -263,8 +263,8 @@ TEST_F(VectorClockMongoDTest, GossipInExternal) { // $configTime or $topologyTime. auto afterTime = vc->getTime(); ASSERT_EQ(afterTime.clusterTime().asTimestamp(), Timestamp(2, 2)); - ASSERT_EQ(afterTime.configTime().asTimestamp(), Timestamp(0, 0)); - ASSERT_EQ(afterTime.topologyTime().asTimestamp(), Timestamp(0, 0)); + ASSERT_EQ(afterTime.configTime(), VectorClock::kInitialComponentTime); + ASSERT_EQ(afterTime.topologyTime(), VectorClock::kInitialComponentTime); vc->gossipIn(nullptr, BSON("$clusterTime" @@ -274,8 +274,8 @@ TEST_F(VectorClockMongoDTest, GossipInExternal) { auto afterTime2 = vc->getTime(); ASSERT_EQ(afterTime2.clusterTime().asTimestamp(), Timestamp(2, 2)); - ASSERT_EQ(afterTime2.configTime().asTimestamp(), Timestamp(0, 0)); - ASSERT_EQ(afterTime2.topologyTime().asTimestamp(), Timestamp(0, 0)); + ASSERT_EQ(afterTime2.configTime(), VectorClock::kInitialComponentTime); + ASSERT_EQ(afterTime2.topologyTime(), VectorClock::kInitialComponentTime); vc->gossipIn(nullptr, BSON("$clusterTime" @@ -285,8 +285,8 @@ TEST_F(VectorClockMongoDTest, GossipInExternal) { auto afterTime3 = vc->getTime(); ASSERT_EQ(afterTime3.clusterTime().asTimestamp(), Timestamp(3, 3)); - ASSERT_EQ(afterTime2.configTime().asTimestamp(), Timestamp(0, 0)); - ASSERT_EQ(afterTime3.topologyTime().asTimestamp(), Timestamp(0, 0)); + ASSERT_EQ(afterTime2.configTime(), VectorClock::kInitialComponentTime); + ASSERT_EQ(afterTime3.topologyTime(), VectorClock::kInitialComponentTime); } } // namespace diff --git a/src/mongo/db/vector_clock_test.cpp b/src/mongo/db/vector_clock_test.cpp index 713a239d006..9d462b85c10 100644 --- a/src/mongo/db/vector_clock_test.cpp +++ b/src/mongo/db/vector_clock_test.cpp @@ -320,7 +320,7 @@ TEST_F(VectorClockTest, RejectsLogicalTimesGreaterThanMaxTime) { resetClock(); ASSERT_THROWS(VectorClockMutable::get(getServiceContext())->tickClusterTimeTo(beyondMaxTime), DBException); - ASSERT_TRUE(getClusterTime() == LogicalTime()); + ASSERT_TRUE(getClusterTime() == VectorClock::kInitialComponentTime); // The time can't be advanced through metadata to a time greater than the max possible. // Advance the wall clock close enough to the new value so the rate check is passed. @@ -328,7 +328,7 @@ TEST_F(VectorClockTest, RejectsLogicalTimesGreaterThanMaxTime) { Seconds(maxVal) - Seconds(kMaxAcceptableLogicalClockDriftSecsDefault) + Seconds(10); setMockClockSourceTime(Date_t::fromDurationSinceEpoch(almostMaxSecs)); ASSERT_THROWS(advanceClusterTime(beyondMaxTime), DBException); - ASSERT_TRUE(getClusterTime() == LogicalTime()); + ASSERT_TRUE(getClusterTime() == VectorClock::kInitialComponentTime); } } // unnamed namespace diff --git a/src/mongo/s/balancer_configuration_test.cpp b/src/mongo/s/balancer_configuration_test.cpp index 89489525182..5933017803f 100644 --- a/src/mongo/s/balancer_configuration_test.cpp +++ b/src/mongo/s/balancer_configuration_test.cpp @@ -39,6 +39,7 @@ #include "mongo/client/remote_command_targeter_mock.h" #include "mongo/db/namespace_string.h" #include "mongo/db/query/query_request_helper.h" +#include "mongo/db/vector_clock.h" #include "mongo/executor/remote_command_request.h" #include "mongo/rpc/metadata/repl_set_metadata.h" #include "mongo/rpc/metadata/tracking_metadata.h" @@ -85,7 +86,9 @@ protected: ASSERT_EQ(findCommand->getNamespaceOrUUID().nss()->ns(), "config.settings"); ASSERT_BSONOBJ_EQ(findCommand->getFilter(), BSON("_id" << key)); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); if (!result.isOK()) { return StatusWith>(result.getStatus()); diff --git a/src/mongo/s/catalog/sharding_catalog_client_test.cpp b/src/mongo/s/catalog/sharding_catalog_client_test.cpp index 63dd880b63f..788b0ae9342 100644 --- a/src/mongo/s/catalog/sharding_catalog_client_test.cpp +++ b/src/mongo/s/catalog/sharding_catalog_client_test.cpp @@ -40,6 +40,7 @@ #include "mongo/db/query/query_request_helper.h" #include "mongo/db/repl/read_concern_args.h" #include "mongo/db/time_proof_service.h" +#include "mongo/db/vector_clock.h" #include "mongo/executor/task_executor.h" #include "mongo/rpc/get_status_from_command_result.h" #include "mongo/rpc/metadata/repl_set_metadata.h" @@ -111,7 +112,9 @@ TEST_F(ShardingCatalogClientTest, GetCollectionExisting) { ASSERT_BSONOBJ_EQ(query->getSort(), BSONObj()); ASSERT_EQ(query->getLimit().get(), 1); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); ReplSetMetadata metadata(10, {newOpTime, Date_t() + Seconds(newOpTime.getSecs())}, @@ -181,7 +184,9 @@ TEST_F(ShardingCatalogClientTest, GetDatabaseExisting) { ASSERT_BSONOBJ_EQ(query->getSort(), BSONObj()); ASSERT(!query->getLimit()); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); ReplSetMetadata metadata(10, {newOpTime, Date_t() + Seconds(newOpTime.getSecs())}, @@ -312,7 +317,9 @@ TEST_F(ShardingCatalogClientTest, GetAllShardsValid) { ASSERT_BSONOBJ_EQ(query->getSort(), BSONObj()); ASSERT_FALSE(query->getLimit().is_initialized()); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); return vector{s1.toBSON(), s2.toBSON(), s3.toBSON()}; }); @@ -414,7 +421,9 @@ TEST_F(ShardingCatalogClientTest, GetChunksForNSWithSortAndLimit) { ASSERT_BSONOBJ_EQ(query->getSort(), BSON(ChunkType::lastmod() << -1)); ASSERT_EQ(query->getLimit().get(), 1); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); ReplSetMetadata metadata(10, {newOpTime, Date_t() + Seconds(newOpTime.getSecs())}, @@ -478,7 +487,9 @@ TEST_F(ShardingCatalogClientTest, GetChunksForUUIDNoSortNoLimit) { ASSERT_BSONOBJ_EQ(query->getSort(), BSONObj()); ASSERT_FALSE(query->getLimit().is_initialized()); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); return vector{}; }); @@ -797,7 +808,9 @@ TEST_F(ShardingCatalogClientTest, GetCollectionsValidResultsNoDb) { ASSERT_BSONOBJ_EQ(query->getFilter(), BSONObj()); ASSERT_BSONOBJ_EQ(query->getSort(), BSONObj()); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); ReplSetMetadata metadata(10, {newOpTime, Date_t() + Seconds(newOpTime.getSecs())}, @@ -850,7 +863,9 @@ TEST_F(ShardingCatalogClientTest, GetCollectionsValidResultsWithDb) { ASSERT_BSONOBJ_EQ(query->getFilter(), b.obj()); } - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); return vector{coll1.toBSON(), coll2.toBSON()}; }); @@ -888,7 +903,9 @@ TEST_F(ShardingCatalogClientTest, GetCollectionsInvalidCollectionType) { ASSERT_BSONOBJ_EQ(query->getFilter(), b.obj()); } - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); return vector{ validColl.toBSON(), @@ -925,7 +942,9 @@ TEST_F(ShardingCatalogClientTest, GetDatabasesForShardValid) { BSON(DatabaseType::primary(dbt1.getPrimary().toString()))); ASSERT_BSONOBJ_EQ(query->getSort(), BSONObj()); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); return vector{dbt1.toBSON(), dbt2.toBSON()}; }); @@ -994,7 +1013,9 @@ TEST_F(ShardingCatalogClientTest, GetTagsForCollection) { ASSERT_BSONOBJ_EQ(query->getFilter(), BSON(TagsType::ns("TestDB.TestColl"))); ASSERT_BSONOBJ_EQ(query->getSort(), BSON(TagsType::min() << 1)); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); return vector{tagA.toBSON(), tagB.toBSON()}; }); @@ -1357,7 +1378,9 @@ TEST_F(ShardingCatalogClientTest, GetNewKeys) { ASSERT_BSONOBJ_EQ(BSON("expiresAt" << 1), query->getSort()); ASSERT_FALSE(query->getLimit().is_initialized()); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); return vector{key1.toBSON(), key2.toBSON()}; }); @@ -1409,7 +1432,9 @@ TEST_F(ShardingCatalogClientTest, GetNewKeysWithEmptyCollection) { ASSERT_BSONOBJ_EQ(BSON("expiresAt" << 1), query->getSort()); ASSERT_FALSE(query->getLimit().is_initialized()); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); return vector{}; }); diff --git a/src/mongo/s/client/shard_remote_test.cpp b/src/mongo/s/client/shard_remote_test.cpp index 3c4cc4181ca..c7b9c1b7975 100644 --- a/src/mongo/s/client/shard_remote_test.cpp +++ b/src/mongo/s/client/shard_remote_test.cpp @@ -32,6 +32,7 @@ #include "mongo/client/connection_string.h" #include "mongo/db/logical_time.h" #include "mongo/db/query/cursor_response.h" +#include "mongo/db/vector_clock.h" #include "mongo/s/catalog/type_shard.h" #include "mongo/s/client/shard_factory.h" #include "mongo/s/client/shard_registry.h" @@ -134,8 +135,8 @@ TEST_F(ShardRemoteTest, NetworkReplyWithLastCommittedOpTime) { // Verify shards that were not targeted were not affected. for (auto shardId : kTestShardIds) { if (shardId != targetedShard) { - ASSERT_EQ(LogicalTime::kUninitialized, - shardRegistry()->getShardNoReload(shardId)->getLastCommittedOpTime()); + ASSERT(!VectorClock::isValidComponentTime( + shardRegistry()->getShardNoReload(shardId)->getLastCommittedOpTime())); } } } diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index 668241285ce..758b6a20c9e 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -152,14 +152,14 @@ void appendRequiredFieldsToResponse(OperationContext* opCtx, BSONObjBuilder* res // Ensure that either both operationTime and $clusterTime are output, or neither. if (clusterTimeWasOutput) { auto operationTime = OperationTimeTracker::get(opCtx)->getMaxOperationTime(); - if (operationTime != LogicalTime::kUninitialized) { + if (VectorClock::isValidComponentTime(operationTime)) { LOGV2_DEBUG(22764, 5, "Appending operationTime: {operationTime}", "Appending operationTime", "operationTime"_attr = operationTime.asTimestamp()); operationTime.appendAsOperationTime(responseBuilder); - } else if (clusterTime != LogicalTime::kUninitialized) { + } else if (VectorClock::isValidComponentTime(clusterTime)) { // If we don't know the actual operation time, use the cluster time instead. This is // safe but not optimal because we can always return a later operation time than // actual. diff --git a/src/mongo/s/sharding_router_test_fixture.cpp b/src/mongo/s/sharding_router_test_fixture.cpp index f3348801a4e..4e9779773ac 100644 --- a/src/mongo/s/sharding_router_test_fixture.cpp +++ b/src/mongo/s/sharding_router_test_fixture.cpp @@ -46,6 +46,7 @@ #include "mongo/db/query/collation/collator_factory_mock.h" #include "mongo/db/query/query_request_helper.h" #include "mongo/db/repl/read_concern_args.h" +#include "mongo/db/vector_clock.h" #include "mongo/db/vector_clock_metadata_hook.h" #include "mongo/executor/task_executor_pool.h" #include "mongo/executor/thread_pool_task_executor_test_fixture.h" @@ -262,7 +263,9 @@ void ShardingTestFixture::expectGetShards(const std::vector& shards) ASSERT_BSONOBJ_EQ(query->getSort(), BSONObj()); ASSERT_FALSE(query->getLimit().is_initialized()); - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); std::vector shardsToReturn; @@ -358,7 +361,9 @@ void ShardingTestFixture::expectCount(const HostAndPort& configHost, return BSON("ok" << 1 << "n" << response.getValue()); } - checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); + checkReadConcern(request.cmdObj, + VectorClock::kInitialComponentTime.asTimestamp(), + repl::OpTime::kUninitializedTerm); BSONObjBuilder responseBuilder; CommandHelpers::appendCommandStatusNoThrow(responseBuilder, response.getStatus()); -- cgit v1.2.1