SERVER-33798 Fail atClusterTime reads at times newer than the current cluster time

4 files changed, 38 insertions, 17 deletions

diff --git a/jstests/noPassthrough/after_cluster_time.js b/jstests/noPassthrough/after_cluster_time.js
index ae1ef3b9e8d..718a9633a0c 100644
--- a/jstests/noPassthrough/after_cluster_time.js
+++ b/jstests/noPassthrough/after_cluster_time.js
@@ -30,15 +30,13 @@
         ErrorCodes.InvalidOptions,
         "expected afterClusterTime read with null timestamp to fail on standalone mongod");
 
-    // Standalones don't store clusterTime, so any non-zero afterClusterTime read value will be
-    // rejected for being ahead of the server's uninitialized internal clusterTime.
-    assert.commandFailedWithCode(
-        testDB.runCommand({
-            find: "after_cluster_time",
-            readConcern: {level: "majority", afterClusterTime: Timestamp(0, 1)}
-        }),
-        ErrorCodes.InvalidOptions,
-        "expected afterClusterTime read with non-zero timestamp to fail on standalone mongod");
+    // Standalones don't support any operations with clusterTime.
+    assert.commandFailedWithCode(testDB.runCommand({
+        find: "after_cluster_time",
+        readConcern: {level: "majority", afterClusterTime: Timestamp(0, 1)}
+    }),
+                                 ErrorCodes.IllegalOperation,
+                                 "expected afterClusterTime read to fail on standalone mongod");
     MongoRunner.stopMongod(standalone);
 
     var rst = new ReplSetTest({nodes: 1});
diff --git a/jstests/noPassthrough/readConcern_atClusterTime.js b/jstests/noPassthrough/readConcern_atClusterTime.js
index f94111b8842..f60a60e349c 100644
--- a/jstests/noPassthrough/readConcern_atClusterTime.js
+++ b/jstests/noPassthrough/readConcern_atClusterTime.js
@@ -43,6 +43,15 @@
         txnNumber: NumberLong(txnNumber++)
     }));
 
+    // 'atClusterTime' cannot be greater than the current cluster time.
+    const futureClusterTime = new Timestamp(clusterTime.getTime() + 1000, 1);
+    assert.commandFailedWithCode(sessionDb.runCommand({
+        find: collName,
+        readConcern: {level: "snapshot", atClusterTime: futureClusterTime},
+        txnNumber: NumberLong(txnNumber++)
+    }),
+                                 ErrorCodes.InvalidOptions);
+
     // 'atClusterTime' must have type Timestamp.
     assert.commandFailedWithCode(sessionDb.runCommand({
         find: collName,
diff --git a/jstests/noPassthrough/readConcern_atClusterTime_noop_write.js b/jstests/noPassthrough/readConcern_atClusterTime_noop_write.js
index ad6bda26619..ebafbb0c1d4 100644
--- a/jstests/noPassthrough/readConcern_atClusterTime_noop_write.js
+++ b/jstests/noPassthrough/readConcern_atClusterTime_noop_write.js
@@ -69,7 +69,7 @@
 
     // Propagate 'clusterTime' to shard 0. This ensures that its next write will be at time >=
     // 'clusterTime'.
-    testDB0.coll0.find().itcount();
+    testDB0.coll0.find().readPref('secondary').itcount();
 
     // Attempt a snapshot read at 'clusterTime' on shard 0. Test that it performs a noop write to
     // advance its majority commit point. The snapshot read itself may fail if the noop write
diff --git a/src/mongo/db/read_concern.cpp b/src/mongo/db/read_concern.cpp
index b5af878c5b8..95350667659 100644
--- a/src/mongo/db/read_concern.cpp
+++ b/src/mongo/db/read_concern.cpp
@@ -255,18 +255,32 @@ Status waitForReadConcern(OperationContext* opCtx,
         if (!allowAfterClusterTime) {
             return {ErrorCodes::InvalidOptions, "afterClusterTime is not allowed for this command"};
         }
-
-        auto currentTime = LogicalClock::get(opCtx)->getClusterTime();
-        if (currentTime < *afterClusterTime) {
-            return {ErrorCodes::InvalidOptions,
-                    "readConcern afterClusterTime must not be greater than clusterTime value"};
-        }
     }
 
     if (!readConcernArgs.isEmpty()) {
         invariant(!afterClusterTime || !atClusterTime);
         auto targetClusterTime = afterClusterTime ? afterClusterTime : atClusterTime;
-        if (replCoord->isReplEnabled() && targetClusterTime) {
+
+        if (targetClusterTime) {
+            std::string readConcernName = afterClusterTime ? "afterClusterTime" : "atClusterTime";
+
+            if (!replCoord->isReplEnabled()) {
+                return {ErrorCodes::IllegalOperation,
+                        str::stream() << "Cannot specify " << readConcernName
+                                      << " readConcern without replication enabled"};
+            }
+
+            auto currentTime = LogicalClock::get(opCtx)->getClusterTime();
+            if (currentTime < *targetClusterTime) {
+                return {ErrorCodes::InvalidOptions,
+                        str::stream() << "readConcern " << readConcernName
+                                      << " value must not be greater than the current clusterTime. "
+                                         "Requested clusterTime: "
+                                      << targetClusterTime->toString()
+                                      << "; current clusterTime: "
+                                      << currentTime.toString()};
+            }
+
             auto status = makeNoopWriteIfNeeded(opCtx, *targetClusterTime);
             if (!status.isOK()) {
                 LOG(0) << "Failed noop write at clusterTime: " << targetClusterTime->toString()