diff options
author | Spencer T Brody <spencer@mongodb.com> | 2018-03-09 19:25:12 -0500 |
---|---|---|
committer | Spencer T Brody <spencer@mongodb.com> | 2018-03-15 19:31:57 -0400 |
commit | 0abcf8f3cf547327c4dab4aea1ff22e1d75a8db8 (patch) | |
tree | 1974c43d6ee2abf8f62ac782ba3054db362b2051 | |
parent | 352cf826069d74ceb6c8a4b4ae2198098c05ba2a (diff) | |
download | mongo-0abcf8f3cf547327c4dab4aea1ff22e1d75a8db8.tar.gz |
SERVER-33798 Fail atClusterTime reads at times newer than the current cluster time
-rw-r--r-- | jstests/noPassthrough/after_cluster_time.js | 16 | ||||
-rw-r--r-- | jstests/noPassthrough/readConcern_atClusterTime.js | 9 | ||||
-rw-r--r-- | jstests/noPassthrough/readConcern_atClusterTime_noop_write.js | 2 | ||||
-rw-r--r-- | src/mongo/db/read_concern.cpp | 28 |
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() |