SERVER-28565 Do not return logicalTime on non-sharded RS

12 files changed, 146 insertions, 42 deletions

diff --git a/src/mongo/db/commands/dbcommands.cpp b/src/mongo/db/commands/dbcommands.cpp
index becd4501056..4dbae9567da 100644
--- a/src/mongo/db/commands/dbcommands.cpp
+++ b/src/mongo/db/commands/dbcommands.cpp
@@ -1321,8 +1321,11 @@ void appendReplyMetadata(OperationContext* opCtx,
                 .writeToMetadata(metadataBob);
         }
 
-        rpc::LogicalTimeMetadata logicalTimeMetadata(LogicalClock::get(opCtx)->getClusterTime());
-        logicalTimeMetadata.writeToMetadata(metadataBob);
+        if (LogicalClock::get(opCtx)->canVerifyAndSign()) {
+            rpc::LogicalTimeMetadata logicalTimeMetadata(
+                LogicalClock::get(opCtx)->getClusterTime());
+            logicalTimeMetadata.writeToMetadata(metadataBob);
+        }
     }
 
     // If we're a shard other than the config shard, attach the last configOpTime we know about.
diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp
index 44aba032a48..68a3b7f0b44 100644
--- a/src/mongo/db/db.cpp
+++ b/src/mongo/db/db.cpp
@@ -894,9 +894,7 @@ MONGO_INITIALIZER_WITH_PREREQUISITES(CreateReplicationManager,
     topoCoordOptions.maxSyncSourceLagSecs = Seconds(repl::maxSyncSourceLagSecs);
     topoCoordOptions.clusterRole = serverGlobalParams.clusterRole;
 
-    auto timeProofService = stdx::make_unique<TimeProofService>();
-    auto logicalClock =
-        stdx::make_unique<LogicalClock>(serviceContext, std::move(timeProofService));
+    auto logicalClock = stdx::make_unique<LogicalClock>(serviceContext);
     LogicalClock::set(serviceContext, std::move(logicalClock));
 
     auto hookList = stdx::make_unique<rpc::EgressMetadataHookList>();
diff --git a/src/mongo/db/logical_clock.cpp b/src/mongo/db/logical_clock.cpp
index 4cdd3177d26..6a4df1e6c60 100644
--- a/src/mongo/db/logical_clock.cpp
+++ b/src/mongo/db/logical_clock.cpp
@@ -84,21 +84,47 @@ void LogicalClock::set(ServiceContext* service, std::unique_ptr<LogicalClock> cl
     clock = std::move(clockArg);
 }
 
-LogicalClock::LogicalClock(ServiceContext* service, std::unique_ptr<TimeProofService> tps)
-    : _service(service), _timeProofService(std::move(tps)) {}
+LogicalClock::LogicalClock(ServiceContext* service) : _service(service) {}
 
 SignedLogicalTime LogicalClock::getClusterTime() {
     stdx::lock_guard<stdx::mutex> lock(_mutex);
     return _clusterTime;
 }
 
-SignedLogicalTime LogicalClock::_makeSignedLogicalTime(LogicalTime logicalTime) {
-    // TODO: SERVER-28436 Implement KeysCollectionManager
-    // Replace dummy keyId with real id from key manager.
-    return SignedLogicalTime(logicalTime, _timeProofService->getProof(logicalTime, _tempKey), 0);
+void LogicalClock::setTimeProofService(std::unique_ptr<TimeProofService> tps) {
+    stdx::lock_guard<stdx::mutex> lock(_mutex);
+    _timeProofService = std::move(tps);
+
+    // Ensure a clock with a time proof service cannot have a cluster time without a proof to
+    // simplify reasoning about signed logical times.
+    if (!_clusterTime.getProof()) {
+        _clusterTime = _makeSignedLogicalTime_inlock(_clusterTime.getTime());
+    }
+}
+
+bool LogicalClock::canVerifyAndSign() {
+    stdx::lock_guard<stdx::mutex> lock(_mutex);
+    return !!_timeProofService;
+}
+
+SignedLogicalTime LogicalClock::_makeSignedLogicalTime_inlock(LogicalTime logicalTime) {
+    if (_timeProofService) {
+        // TODO: SERVER-28436 Implement KeysCollectionManager
+        // Replace dummy keyId with real id from key manager.
+        return SignedLogicalTime(
+            logicalTime, _timeProofService->getProof(logicalTime, _tempKey), 0);
+    }
+    return SignedLogicalTime(logicalTime, 0);
 }
 
 Status LogicalClock::advanceClusterTime(const SignedLogicalTime& newTime) {
+    stdx::lock_guard<stdx::mutex> lock(_mutex);
+    if (!_timeProofService) {
+        return Status(ErrorCodes::CannotVerifyAndSignLogicalTime,
+                      "Cannot accept logicalTime: " + newTime.getTime().toString() +
+                          ". May not be a part of a sharded cluster");
+    }
+
     const auto& newLogicalTime = newTime.getTime();
 
     // No need to check proof if no time was given.
@@ -106,13 +132,24 @@ Status LogicalClock::advanceClusterTime(const SignedLogicalTime& newTime) {
         return Status::OK();
     }
 
-    invariant(_timeProofService);
-    auto res = _timeProofService->checkProof(newLogicalTime, newTime.getProof(), _tempKey);
+    const auto newProof = newTime.getProof();
+    // Logical time is only sent if a server's clock can verify and sign logical times, so any
+    // received logical times should have proofs.
+    invariant(newProof);
+
+    auto res = _timeProofService->checkProof(newLogicalTime, newProof.get(), _tempKey);
     if (res != Status::OK()) {
         return res;
     }
 
-    return advanceClusterTimeFromTrustedSource(newTime);
+    // The rate limiter check cannot be moved into _advanceClusterTime_inlock to avoid code
+    // repetition because it shouldn't be called on direct oplog operations.
+    auto rateLimitStatus = _passesRateLimiter_inlock(newTime.getTime());
+    if (!rateLimitStatus.isOK()) {
+        return rateLimitStatus;
+    }
+
+    return _advanceClusterTime_inlock(std::move(newTime));
 }
 
 Status LogicalClock::_advanceClusterTime_inlock(SignedLogicalTime newTime) {
@@ -125,6 +162,8 @@ Status LogicalClock::_advanceClusterTime_inlock(SignedLogicalTime newTime) {
 
 Status LogicalClock::advanceClusterTimeFromTrustedSource(SignedLogicalTime newTime) {
     stdx::lock_guard<stdx::mutex> lock(_mutex);
+    // The rate limiter check cannot be moved into _advanceClusterTime_inlock to avoid code
+    // repetition because it shouldn't be called on direct oplog operations.
     auto rateLimitStatus = _passesRateLimiter_inlock(newTime.getTime());
     if (!rateLimitStatus.isOK()) {
         return rateLimitStatus;
@@ -134,9 +173,9 @@ Status LogicalClock::advanceClusterTimeFromTrustedSource(SignedLogicalTime newTi
 }
 
 Status LogicalClock::signAndAdvanceClusterTime(LogicalTime newTime) {
-    auto newSignedTime = _makeSignedLogicalTime(newTime);
-
     stdx::lock_guard<stdx::mutex> lock(_mutex);
+    auto newSignedTime = _makeSignedLogicalTime_inlock(newTime);
+
     return _advanceClusterTime_inlock(std::move(newSignedTime));
 }
 
@@ -180,15 +219,16 @@ LogicalTime LogicalClock::reserveTicks(uint64_t nTicks) {
         clusterTime.addTicks(nTicks - 1);
     }
 
-    _clusterTime = _makeSignedLogicalTime(clusterTime);
+    _clusterTime = _makeSignedLogicalTime_inlock(clusterTime);
     return nextClusterTime;
 }
 
 void LogicalClock::initClusterTimeFromTrustedSource(LogicalTime newTime) {
+    stdx::lock_guard<stdx::mutex> lock(_mutex);
     invariant(_clusterTime.getTime() == LogicalTime::kUninitialized);
     // Rate limit checks are skipped here so a server with no activity for longer than
     // maxAcceptableLogicalClockDrift seconds can still have its cluster time initialized.
-    _clusterTime = _makeSignedLogicalTime(newTime);
+    _clusterTime = _makeSignedLogicalTime_inlock(newTime);
 }
 
 Status LogicalClock::_passesRateLimiter_inlock(LogicalTime newTime) {
diff --git a/src/mongo/db/logical_clock.h b/src/mongo/db/logical_clock.h
index 0a4cb99f5fb..001d91eb9e6 100644
--- a/src/mongo/db/logical_clock.h
+++ b/src/mongo/db/logical_clock.h
@@ -54,9 +54,20 @@ public:
         Seconds(365 * 24 * 60 * 60);  // 1 year
 
     /**
-     *  Creates an instance of LogicalClock. The TimeProofService must already be fully initialized.
+     *  Creates an instance of LogicalClock.
      */
-    LogicalClock(ServiceContext*, std::unique_ptr<TimeProofService>);
+    LogicalClock(ServiceContext*);
+
+    /**
+     * Attach a pointer to a TimeProofService to the logical clock. Will overwrite an existing
+     * pointer if a TimeProofService has already been attached.
+     */
+    void setTimeProofService(std::unique_ptr<TimeProofService>);
+
+    /**
+     * Returns true if a TimeProofService has been attached to the logical clock.
+     */
+    bool canVerifyAndSign();
 
     /**
      * The method sets clusterTime to the newTime if the newTime > _clusterTime and the newTime
@@ -67,7 +78,7 @@ public:
     Status advanceClusterTime(const SignedLogicalTime&);
 
     /**
-     * Similar to advaneClusterTime, but only does rate checking and not proof validation.
+     * Similar to advanceClusterTime, but only does rate checking and not proof validation.
      */
     Status advanceClusterTimeFromTrustedSource(SignedLogicalTime newTime);
 
@@ -100,7 +111,7 @@ private:
     /**
      * Utility to create valid SignedLogicalTime from LogicalTime.
      */
-    SignedLogicalTime _makeSignedLogicalTime(LogicalTime);
+    SignedLogicalTime _makeSignedLogicalTime_inlock(LogicalTime);
 
     Status _advanceClusterTime_inlock(SignedLogicalTime newTime);
 
@@ -111,11 +122,11 @@ private:
     Status _passesRateLimiter_inlock(LogicalTime newTime);
 
     ServiceContext* const _service;
-    std::unique_ptr<TimeProofService> _timeProofService;
 
-    // the mutex protects _clusterTime
+    // The mutex protects _clusterTime and _timeProofService.
     stdx::mutex _mutex;
     SignedLogicalTime _clusterTime;
+    std::unique_ptr<TimeProofService> _timeProofService;
 
     /**
      * Temporary key only used for unit tests.
diff --git a/src/mongo/db/logical_clock_test.cpp b/src/mongo/db/logical_clock_test.cpp
index e5b429b776a..8b28cc06bd4 100644
--- a/src/mongo/db/logical_clock_test.cpp
+++ b/src/mongo/db/logical_clock_test.cpp
@@ -52,7 +52,8 @@ protected:
         _serviceContext = stdx::make_unique<ServiceContextNoop>();
         auto pTps = stdx::make_unique<TimeProofService>();
         _timeProofService = pTps.get();
-        _clock = stdx::make_unique<LogicalClock>(_serviceContext.get(), std::move(pTps));
+        _clock = stdx::make_unique<LogicalClock>(_serviceContext.get());
+        _clock->setTimeProofService(std::move(pTps));
         _serviceContext->setFastClockSource(
             stdx::make_unique<SharedClockSourceAdapter>(_mockClockSource));
     }
@@ -84,6 +85,16 @@ protected:
             _serviceContext->getFastClockSource()->now().toDurationSinceEpoch());
     }
 
+    void resetTimeProofService() {
+        auto pTps = stdx::make_unique<TimeProofService>();
+        _timeProofService = pTps.get();
+        _clock->setTimeProofService(std::move(pTps));
+    }
+
+    void unsetTimeProofService() {
+        _clock->setTimeProofService(std::unique_ptr<TimeProofService>());
+    }
+
 private:
     TimeProofService* _timeProofService;
     std::unique_ptr<ServiceContextNoop> _serviceContext;
@@ -169,5 +180,43 @@ TEST_F(LogicalClockTestBase, InitFromTrustedSourceCanAcceptVeryOldLogicalTime) {
     ASSERT_TRUE(getClock()->getClusterTime().getTime() == veryOldTime);
 }
 
+// A clock with no TimeProofService should reject new times in advanceClusterTime.
+TEST_F(LogicalClockTestBase, AdvanceClusterTimeFailsWithoutTimeProofService) {
+    LogicalTime initialTime(Timestamp(10));
+    getClock()->initClusterTimeFromTrustedSource(initialTime);
+
+    unsetTimeProofService();
+
+    SignedLogicalTime l1 = makeSignedLogicalTime(LogicalTime(Timestamp(100)));
+    ASSERT_EQ(ErrorCodes::CannotVerifyAndSignLogicalTime, getClock()->advanceClusterTime(l1));
+    ASSERT_TRUE(getClock()->getClusterTime().getTime() == initialTime);
+
+    resetTimeProofService();
+
+    SignedLogicalTime l2 = makeSignedLogicalTime(LogicalTime(Timestamp(200)));
+    ASSERT_OK(getClock()->advanceClusterTime(l2));
+    ASSERT_TRUE(getClock()->getClusterTime().getTime() == l2.getTime());
+}
+
+// A clock with no TimeProofService can still advance its time through certain methods.
+TEST_F(LogicalClockTestBase, CertainMethodsCanAdvanceClockWithoutTimeProofService) {
+    unsetTimeProofService();
+
+    LogicalTime t1(Timestamp(100));
+    getClock()->initClusterTimeFromTrustedSource(t1);
+    ASSERT_TRUE(getClock()->getClusterTime().getTime() == t1);
+
+    auto t2 = getClock()->reserveTicks(1);
+    ASSERT_TRUE(getClock()->getClusterTime().getTime() == t2);
+
+    LogicalTime t3(Timestamp(300));
+    ASSERT_OK(getClock()->signAndAdvanceClusterTime(t3));
+    ASSERT_TRUE(getClock()->getClusterTime().getTime() == t3);
+
+    SignedLogicalTime l4 = makeSignedLogicalTime(LogicalTime(Timestamp(400)));
+    ASSERT_OK(getClock()->advanceClusterTimeFromTrustedSource(l4));
+    ASSERT_TRUE(getClock()->getClusterTime().getTime() == l4.getTime());
+}
+
 }  // unnamed namespace
 }  // namespace mongo
diff --git a/src/mongo/db/logical_clock_test_fixture.cpp b/src/mongo/db/logical_clock_test_fixture.cpp
index 6f5860b7a66..a8cee37303a 100644
--- a/src/mongo/db/logical_clock_test_fixture.cpp
+++ b/src/mongo/db/logical_clock_test_fixture.cpp
@@ -32,7 +32,6 @@
 
 #include "mongo/db/logical_clock.h"
 #include "mongo/db/service_context.h"
-#include "mongo/db/time_proof_service.h"
 #include "mongo/stdx/memory.h"
 
 namespace mongo {
@@ -40,8 +39,7 @@ namespace mongo {
 void LogicalClockTest::setUp() {
     auto service = getGlobalServiceContext();
 
-    auto timeProofService = stdx::make_unique<TimeProofService>();
-    auto logicalClock = stdx::make_unique<LogicalClock>(service, std::move(timeProofService));
+    auto logicalClock = stdx::make_unique<LogicalClock>(service);
     LogicalClock::set(service, std::move(logicalClock));
 }
 
diff --git a/src/mongo/db/logical_time_metadata_hook.cpp b/src/mongo/db/logical_time_metadata_hook.cpp
index f45e4fba9db..bb4124d611b 100644
--- a/src/mongo/db/logical_time_metadata_hook.cpp
+++ b/src/mongo/db/logical_time_metadata_hook.cpp
@@ -42,6 +42,10 @@ LogicalTimeMetadataHook::LogicalTimeMetadataHook(ServiceContext* service) : _ser
 
 Status LogicalTimeMetadataHook::writeRequestMetadata(OperationContext* opCtx,
                                                      BSONObjBuilder* metadataBob) {
+    if (!LogicalClock::get(_service)->canVerifyAndSign()) {
+        return Status::OK();
+    }
+
     LogicalTimeMetadata metadata(LogicalClock::get(_service)->getClusterTime());
     metadata.writeToMetadata(metadataBob);
     return Status::OK();
@@ -54,6 +58,12 @@ Status LogicalTimeMetadataHook::readReplyMetadata(StringData replySource,
         return parseStatus.getStatus();
     }
     auto& signedTime = parseStatus.getValue().getSignedTime();
+    // LogicalTimeMetadata is default constructed if no logical time metadata was sent, so a
+    // default constructed SignedLogicalTime should be ignored.
+    if (signedTime.getTime() == LogicalTime::kUninitialized) {
+        return Status::OK();
+    }
+
     return LogicalClock::get(_service)->advanceClusterTimeFromTrustedSource(signedTime);
 }
 
diff --git a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
index 44cde2d8db7..43b4a9428c2 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
@@ -739,10 +739,6 @@ void ReplicationCoordinatorExternalStateImpl::_shardingOnTransitionToPrimaryHook
 
         // If this is a config server node becoming a primary, start the balancer
         Balancer::get(opCtx)->initiateBalancer(opCtx);
-
-        // Generate and upsert random 20 byte key for the LogicalClock's TimeProofService.
-        // TODO: SERVER-27768
-
     } else if (ShardingState::get(opCtx)->enabled()) {
         const auto configsvrConnStr =
             Grid::get(opCtx)->shardRegistry()->getConfigShard()->getConnString();
diff --git a/src/mongo/db/repl/replication_coordinator_test_fixture.cpp b/src/mongo/db/repl/replication_coordinator_test_fixture.cpp
index 4e241ee07ae..0df971091d4 100644
--- a/src/mongo/db/repl/replication_coordinator_test_fixture.cpp
+++ b/src/mongo/db/repl/replication_coordinator_test_fixture.cpp
@@ -42,7 +42,6 @@
 #include "mongo/db/repl/replication_coordinator_impl.h"
 #include "mongo/db/repl/storage_interface_mock.h"
 #include "mongo/db/repl/topology_coordinator_impl.h"
-#include "mongo/db/time_proof_service.h"
 #include "mongo/executor/network_interface_mock.h"
 #include "mongo/stdx/functional.h"
 #include "mongo/stdx/memory.h"
@@ -124,8 +123,7 @@ void ReplCoordTest::init() {
     // PRNG seed for tests.
     const int64_t seed = 0;
 
-    auto timeProofService = stdx::make_unique<TimeProofService>();
-    auto logicalClock = stdx::make_unique<LogicalClock>(service, std::move(timeProofService));
+    auto logicalClock = stdx::make_unique<LogicalClock>(service);
     LogicalClock::set(service, std::move(logicalClock));
 
     TopologyCoordinatorImpl::Options settings;
diff --git a/src/mongo/db/service_context_d_test_fixture.cpp b/src/mongo/db/service_context_d_test_fixture.cpp
index 49efce677fb..dd18629eb05 100644
--- a/src/mongo/db/service_context_d_test_fixture.cpp
+++ b/src/mongo/db/service_context_d_test_fixture.cpp
@@ -43,7 +43,6 @@
 #include "mongo/db/service_context.h"
 #include "mongo/db/service_context_d.h"
 #include "mongo/db/storage/storage_options.h"
-#include "mongo/db/time_proof_service.h"
 #include "mongo/stdx/memory.h"
 #include "mongo/unittest/temp_dir.h"
 #include "mongo/util/assert_util.h"
@@ -55,9 +54,7 @@ void ServiceContextMongoDTest::setUp() {
     Client::initThread(getThreadName());
     ServiceContext* serviceContext = getServiceContext();
 
-    auto timeProofService = stdx::make_unique<TimeProofService>();
-    auto logicalClock =
-        stdx::make_unique<LogicalClock>(serviceContext, std::move(timeProofService));
+    auto logicalClock = stdx::make_unique<LogicalClock>(serviceContext);
     LogicalClock::set(serviceContext, std::move(logicalClock));
 
     if (!serviceContext->getGlobalStorageEngine()) {
diff --git a/src/mongo/db/signed_logical_time.cpp b/src/mongo/db/signed_logical_time.cpp
index 16404950ac5..dc8f7c3d0be 100644
--- a/src/mongo/db/signed_logical_time.cpp
+++ b/src/mongo/db/signed_logical_time.cpp
@@ -34,7 +34,8 @@ namespace mongo {
 
 std::string SignedLogicalTime::toString() const {
     StringBuilder buf;
-    buf << _time.toString() << "|" << _proof.toString();
+    auto proof = _proof.get_value_or(TimeProof());
+    buf << _time.toString() << "|" << proof.toString();
     return buf.str();
 }
 
diff --git a/src/mongo/db/signed_logical_time.h b/src/mongo/db/signed_logical_time.h
index fde222efb39..caf7830fb32 100644
--- a/src/mongo/db/signed_logical_time.h
+++ b/src/mongo/db/signed_logical_time.h
@@ -44,6 +44,9 @@ public:
 
     SignedLogicalTime() = default;
 
+    explicit SignedLogicalTime(LogicalTime time, long long keyId)
+        : _time(std::move(time)), _keyId(keyId) {}
+
     explicit SignedLogicalTime(LogicalTime time, TimeProof proof, long long keyId)
         : _time(std::move(time)), _proof(std::move(proof)), _keyId(keyId) {}
 
@@ -51,7 +54,7 @@ public:
         return _time;
     }
 
-    const TimeProof& getProof() const {
+    boost::optional<TimeProof> getProof() const {
         return _proof;
     }
 
@@ -65,7 +68,7 @@ public:
 
 private:
     LogicalTime _time;
-    TimeProof _proof;
+    boost::optional<TimeProof> _proof;
     long long _keyId{0};
 };