SERVER-18588 Adjust read-after-optime interface

6 files changed, 29 insertions, 157 deletions

diff --git a/src/mongo/base/error_codes.err b/src/mongo/base/error_codes.err
index e08c7cbfca1..01cc72f601a 100644
--- a/src/mongo/base/error_codes.err
+++ b/src/mongo/base/error_codes.err
@@ -121,7 +121,7 @@ error_code("NamespaceNotSharded", 118)
 error_code("InvalidSyncSource", 119)
 error_code("OplogStartMissing", 120)
 error_code("DocumentValidationFailure", 121) # Only for the document validator on collections.
-error_code("ReadAfterOptimeTimeout", 122)
+error_code("OBSOLETE_ReadAfterOptimeTimeout", 122)
 error_code("NotAReplicaSet", 123)
 error_code("IncompatibleElectionProtocol", 124)
 error_code("CommandFailed", 125)
diff --git a/src/mongo/db/repl/read_after_optime_args.cpp b/src/mongo/db/repl/read_after_optime_args.cpp
index b17c24f23fa..2a7c7817662 100644
--- a/src/mongo/db/repl/read_after_optime_args.cpp
+++ b/src/mongo/db/repl/read_after_optime_args.cpp
@@ -41,28 +41,22 @@ using std::string;
 namespace mongo {
 namespace repl {
 
-    const string ReadAfterOpTimeArgs::kRootFieldName("after");
-    const string ReadAfterOpTimeArgs::kOpTimeFieldName("opTime");
+    const string ReadAfterOpTimeArgs::kRootFieldName("$readConcern");
+    const string ReadAfterOpTimeArgs::kOpTimeFieldName("afterOpTime");
     const string ReadAfterOpTimeArgs::kOpTimestampFieldName("ts");
     const string ReadAfterOpTimeArgs::kOpTermFieldName("term");
-    const string ReadAfterOpTimeArgs::kTimeoutFieldName("timeoutMS");
 
-    ReadAfterOpTimeArgs::ReadAfterOpTimeArgs(): ReadAfterOpTimeArgs(OpTime(), Milliseconds(0)) {
+    ReadAfterOpTimeArgs::ReadAfterOpTimeArgs(): ReadAfterOpTimeArgs(OpTime()) {
     }
 
-    ReadAfterOpTimeArgs::ReadAfterOpTimeArgs(OpTime opTime, Milliseconds timeout):
-            _opTime(std::move(opTime)),
-            _timeout(std::move(timeout)) {
+    ReadAfterOpTimeArgs::ReadAfterOpTimeArgs(OpTime opTime):
+            _opTime(std::move(opTime)) {
     }
 
     const OpTime& ReadAfterOpTimeArgs::getOpTime() const {
         return _opTime;
     }
 
-    const Milliseconds& ReadAfterOpTimeArgs::getTimeout() const {
-        return _timeout;
-    }
-
     Status ReadAfterOpTimeArgs::initialize(const BSONObj& cmdObj) {
         auto afterElem = cmdObj[ReadAfterOpTimeArgs::kRootFieldName];
 
@@ -89,9 +83,10 @@ namespace repl {
         BSONElement timestampElem;
 
         Timestamp timestamp;
-        auto timestampStatus = bsonExtractTimestampField(opTimeObj,
-                                                         ReadAfterOpTimeArgs::kOpTimestampFieldName,
-                                                         &timestamp);
+        auto timestampStatus =
+                bsonExtractTimestampField(opTimeObj,
+                                          ReadAfterOpTimeArgs::kOpTimestampFieldName,
+                                          &timestamp);
 
         if (!timestampStatus.isOK()) {
             return timestampStatus;
@@ -106,26 +101,7 @@ namespace repl {
             return termStatus;
         }
 
-        long long timeoutMS;
-        auto timeoutStatus = bsonExtractIntegerFieldWithDefault(
-                readAfterObj,
-                ReadAfterOpTimeArgs::kTimeoutFieldName,
-                0, // Default to no timeout.
-                &timeoutMS);
-
-        if (!timeoutStatus.isOK()) {
-            return timeoutStatus;
-        }
-
-        if (timeoutMS < 0) {
-            return Status(ErrorCodes::BadValue,
-                          str::stream() << ReadAfterOpTimeArgs::kRootFieldName
-                                        << "." << ReadAfterOpTimeArgs::kTimeoutFieldName
-                                        << " value must be positive");
-        }
-
         _opTime = OpTime(timestamp, termNumber);
-        _timeout = Milliseconds(timeoutMS); // Note: 'long long' -> 'long' down casting.
 
         return Status::OK();
     }
diff --git a/src/mongo/db/repl/read_after_optime_args.h b/src/mongo/db/repl/read_after_optime_args.h
index a8703ad52f7..ee58dd3b9b2 100644
--- a/src/mongo/db/repl/read_after_optime_args.h
+++ b/src/mongo/db/repl/read_after_optime_args.h
@@ -47,19 +47,17 @@ namespace repl {
         static const std::string kOpTimeFieldName;
         static const std::string kOpTimestampFieldName;
         static const std::string kOpTermFieldName;
-        static const std::string kTimeoutFieldName;
 
         ReadAfterOpTimeArgs();
-        ReadAfterOpTimeArgs(OpTime opTime, Milliseconds timeout);
+        explicit ReadAfterOpTimeArgs(OpTime opTime);
 
         /**
          * Format:
          * {
          *    find: “coll”,
          *    filter: <Query Object>,
-         *    after: { // optional
-         *      opTime: { ts: <timestamp>, term: <NumberLong> },
-         *      timeoutMS: <NumberLong> //optional
+         *    $readConcern: { // optional
+         *      afterOpTime: { ts: <timestamp>, term: <NumberLong> },
          *    }
          * }
          */
@@ -71,7 +69,6 @@ namespace repl {
     private:
 
         OpTime _opTime;
-        Milliseconds _timeout;
     };
 
 } // namespace repl
diff --git a/src/mongo/db/repl/read_after_optime_args_test.cpp b/src/mongo/db/repl/read_after_optime_args_test.cpp
index 557767c2886..b37ca9f16ac 100644
--- a/src/mongo/db/repl/read_after_optime_args_test.cpp
+++ b/src/mongo/db/repl/read_after_optime_args_test.cpp
@@ -41,12 +41,10 @@ namespace {
                 << ReadAfterOpTimeArgs::kRootFieldName
                 << BSON(ReadAfterOpTimeArgs::kOpTimeFieldName
                         << BSON(ReadAfterOpTimeArgs::kOpTimestampFieldName << Timestamp(20, 30)
-                                << ReadAfterOpTimeArgs::kOpTermFieldName << 2)
-                << ReadAfterOpTimeArgs::kTimeoutFieldName << 100))));
+                                << ReadAfterOpTimeArgs::kOpTermFieldName << 2)))));
 
         ASSERT_EQ(Timestamp(20, 30), readAfterOpTime.getOpTime().getTimestamp());
         ASSERT_EQ(2, readAfterOpTime.getOpTime().getTerm());
-        ASSERT_EQ(Milliseconds(100), readAfterOpTime.getTimeout());
     }
 
     TEST(ReadAfterParse, Empty) {
@@ -54,7 +52,6 @@ namespace {
         ASSERT_OK(readAfterOpTime.initialize(BSON("find" << "test")));
 
         ASSERT(readAfterOpTime.getOpTime().getTimestamp().isNull());
-        ASSERT_EQ(Milliseconds::zero(), readAfterOpTime.getTimeout());
     }
 
     TEST(ReadAfterParse, BadRootType) {
@@ -76,8 +73,7 @@ namespace {
         ReadAfterOpTimeArgs readAfterOpTime;
         ASSERT_NOT_OK(readAfterOpTime.initialize(BSON(
                 "find" << "test"
-                << ReadAfterOpTimeArgs::kRootFieldName
-                << BSON(ReadAfterOpTimeArgs::kTimeoutFieldName << 100))));
+                << ReadAfterOpTimeArgs::kRootFieldName << BSONObj())));
     }
 
     TEST(ReadAfterParse, NoOpTimeTS) {
@@ -86,8 +82,7 @@ namespace {
                     "find" << "test"
                     << ReadAfterOpTimeArgs::kRootFieldName
                     << BSON(ReadAfterOpTimeArgs::kOpTimeFieldName
-                            << BSON(ReadAfterOpTimeArgs::kOpTermFieldName << 2)
-                    << ReadAfterOpTimeArgs::kTimeoutFieldName << 100))));
+                            << BSON(ReadAfterOpTimeArgs::kOpTermFieldName << 2)))));
     }
 
     TEST(ReadAfterParse, NoOpTimeTerm) {
@@ -96,8 +91,7 @@ namespace {
                 "find" << "test"
                 << ReadAfterOpTimeArgs::kRootFieldName
                 << BSON(ReadAfterOpTimeArgs::kOpTimeFieldName
-                        << BSON(ReadAfterOpTimeArgs::kOpTermFieldName << 2)
-                << ReadAfterOpTimeArgs::kTimeoutFieldName << 100))));
+                        << BSON(ReadAfterOpTimeArgs::kOpTermFieldName << 2)))));
     }
 
     TEST(ReadAfterParse, BadOpTimeTSType) {
@@ -107,70 +101,17 @@ namespace {
                 << ReadAfterOpTimeArgs::kRootFieldName
                 << BSON(ReadAfterOpTimeArgs::kOpTimeFieldName
                         << BSON(ReadAfterOpTimeArgs::kOpTimestampFieldName << BSON("x" << 1)
-                                << ReadAfterOpTimeArgs::kOpTermFieldName << 2)
-                << ReadAfterOpTimeArgs::kTimeoutFieldName << 100))));
-    }
-
-    TEST(ReadAfterParse, BadOpTimeTermType) {
-        ReadAfterOpTimeArgs readAfterOpTime;
-        ASSERT_NOT_OK(readAfterOpTime.initialize(BSON(
-                "find" << "test"
-                << ReadAfterOpTimeArgs::kRootFieldName
-                << BSON(ReadAfterOpTimeArgs::kOpTimeFieldName
-                        << BSON(ReadAfterOpTimeArgs::kOpTimestampFieldName << Timestamp(1, 0)
-                                << ReadAfterOpTimeArgs::kOpTermFieldName << "y")
-                << ReadAfterOpTimeArgs::kTimeoutFieldName << 100))));
-    }
-
-    TEST(ReadAfterParse, TimeoutDefault) {
-        ReadAfterOpTimeArgs readAfterOpTime;
-        ASSERT_OK(readAfterOpTime.initialize(BSON(
-                "find" << "test"
-                << ReadAfterOpTimeArgs::kRootFieldName
-                << BSON(ReadAfterOpTimeArgs::kOpTimeFieldName
-                        << BSON(ReadAfterOpTimeArgs::kOpTimestampFieldName << Timestamp(1, 0)
                                 << ReadAfterOpTimeArgs::kOpTermFieldName << 2)))));
-
-        ASSERT_EQ(Timestamp(1, 0), readAfterOpTime.getOpTime().getTimestamp());
-        ASSERT_EQ(2, readAfterOpTime.getOpTime().getTerm());
-        ASSERT_EQ(Milliseconds::zero(), readAfterOpTime.getTimeout());
     }
 
-    TEST(ReadAfterParse, BadTimeoutType) {
-        ReadAfterOpTimeArgs readAfterOpTime;
-        ASSERT_NOT_OK(readAfterOpTime.initialize(BSON(
-                "find" << "test"
-                << ReadAfterOpTimeArgs::kRootFieldName
-                << BSON(ReadAfterOpTimeArgs::kOpTimeFieldName
-                        << BSON(ReadAfterOpTimeArgs::kOpTimestampFieldName << Timestamp(1, 0)
-                                << ReadAfterOpTimeArgs::kOpTermFieldName << 2)
-                << ReadAfterOpTimeArgs::kTimeoutFieldName << "abc"))));
-    }
-
-    TEST(ReadAfterParse, NegativeTimeout) {
+    TEST(ReadAfterParse, BadOpTimeTermType) {
         ReadAfterOpTimeArgs readAfterOpTime;
         ASSERT_NOT_OK(readAfterOpTime.initialize(BSON(
                 "find" << "test"
                 << ReadAfterOpTimeArgs::kRootFieldName
                 << BSON(ReadAfterOpTimeArgs::kOpTimeFieldName
                         << BSON(ReadAfterOpTimeArgs::kOpTimestampFieldName << Timestamp(1, 0)
-                                << ReadAfterOpTimeArgs::kOpTermFieldName << 2)
-                << ReadAfterOpTimeArgs::kTimeoutFieldName << -100))));
-    }
-
-    TEST(ReadAfterParse, ZeroTimeout) {
-        ReadAfterOpTimeArgs readAfterOpTime;
-        ASSERT_OK(readAfterOpTime.initialize(BSON(
-                "find" << "test"
-                << ReadAfterOpTimeArgs::kRootFieldName
-                << BSON(ReadAfterOpTimeArgs::kOpTimeFieldName
-                        << BSON(ReadAfterOpTimeArgs::kOpTimestampFieldName << Timestamp(20, 30)
-                                << ReadAfterOpTimeArgs::kOpTermFieldName << 2)
-                << ReadAfterOpTimeArgs::kTimeoutFieldName << 0))));
-
-        ASSERT_EQ(Timestamp(20, 30), readAfterOpTime.getOpTime().getTimestamp());
-        ASSERT_EQ(2, readAfterOpTime.getOpTime().getTerm());
-        ASSERT_EQ(Milliseconds::zero(), readAfterOpTime.getTimeout());
+                                << ReadAfterOpTimeArgs::kOpTermFieldName << "y")))));
     }
 
 } // unnamed namespace
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index a2d74635e1c..bd1378699ad 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -776,7 +776,6 @@ namespace {
             OperationContext* txn,
             const ReadAfterOpTimeArgs& settings) {
         const auto& ts = settings.getOpTime();
-        const auto& timeout = settings.getTimeout();
 
         if (ts.isNull()) {
             return ReadAfterOpTimeResponse();
@@ -811,15 +810,6 @@ namespace {
                         Milliseconds(timer.millis()));
             }
 
-            const Microseconds elapsedTime{timer.micros()};
-            if (timeout > Microseconds::zero() && elapsedTime > timeout) {
-                return ReadAfterOpTimeResponse(
-                        Status(ErrorCodes::ReadAfterOptimeTimeout,
-                              str::stream() << "timed out waiting for opTime: "
-                                            << ts.toString()),
-                        duration_cast<Milliseconds>(elapsedTime));
-            }
-
             stdx::condition_variable condVar;
             WaiterInfo waitInfo(&_opTimeWaiterList,
                                 txn->getOpID(),
@@ -827,19 +817,11 @@ namespace {
                                 nullptr, // Don't care about write concern.
                                 &condVar);
 
-            const Microseconds maxTimeMicrosRemaining{txn->getRemainingMaxTimeMicros()};
-            Microseconds waitTime = Microseconds::max();
-            if (maxTimeMicrosRemaining != Microseconds::zero()) {
-                waitTime = maxTimeMicrosRemaining;
-            }
-            if (timeout != Microseconds::zero()) {
-                waitTime = std::min<Microseconds>(timeout - elapsedTime, waitTime);
-            }
-            if (waitTime == Microseconds::max()) {
-                condVar.wait(lock);
+            if (CurOp::get(txn)->isMaxTimeSet()) {
+                condVar.wait_for(lock, Microseconds(txn->getRemainingMaxTimeMicros()));
             }
             else {
-                condVar.wait_for(lock, waitTime);
+                condVar.wait(lock);
             }
         }
 
diff --git a/src/mongo/db/repl/replication_coordinator_impl_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_test.cpp
index e330a1067d2..b625f45fe07 100644
--- a/src/mongo/db/repl/replication_coordinator_impl_test.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl_test.cpp
@@ -1961,8 +1961,7 @@ namespace {
         init(ReplSettings());
         OperationContextNoop txn;
         auto result = getReplCoord()->waitUntilOpTime(&txn,
-                ReadAfterOpTimeArgs(OpTimeWithTermZero(50, 0),
-                                    Milliseconds(0)));
+                ReadAfterOpTimeArgs(OpTimeWithTermZero(50, 0)));
 
         ASSERT_FALSE(result.didWait());
         ASSERT_EQUALS(ErrorCodes::NotAReplicaSet, result.getStatus());
@@ -1981,8 +1980,7 @@ namespace {
         shutdown();
 
         auto result = getReplCoord()->waitUntilOpTime(&txn,
-                ReadAfterOpTimeArgs(OpTimeWithTermZero(50, 0),
-                                    Milliseconds(0)));
+                ReadAfterOpTimeArgs(OpTimeWithTermZero(50, 0)));
 
         ASSERT_TRUE(result.didWait());
         ASSERT_EQUALS(ErrorCodes::ShutdownInProgress, result.getStatus());
@@ -2001,8 +1999,7 @@ namespace {
         txn.setCheckForInterruptStatus(Status(ErrorCodes::Interrupted, "test"));
 
         auto result = getReplCoord()->waitUntilOpTime(&txn,
-                ReadAfterOpTimeArgs(OpTimeWithTermZero(50, 0),
-                                    Milliseconds(0)));
+                ReadAfterOpTimeArgs(OpTimeWithTermZero(50, 0)));
 
         ASSERT_TRUE(result.didWait());
         ASSERT_EQUALS(ErrorCodes::Interrupted, result.getStatus());
@@ -2032,8 +2029,7 @@ namespace {
 
         getReplCoord()->setMyLastOptime(OpTimeWithTermZero(100, 0));
         auto result = getReplCoord()->waitUntilOpTime(&txn,
-                ReadAfterOpTimeArgs(OpTimeWithTermZero(50, 0),
-                                    Milliseconds(100)));
+                ReadAfterOpTimeArgs(OpTimeWithTermZero(50, 0)));
 
         ASSERT_TRUE(result.didWait());
         ASSERT_OK(result.getStatus());
@@ -2050,8 +2046,7 @@ namespace {
 
         OpTimeWithTermZero time(100, 0);
         getReplCoord()->setMyLastOptime(time);
-        auto result = getReplCoord()->waitUntilOpTime(&txn,
-                ReadAfterOpTimeArgs(time, Milliseconds(100)));
+        auto result = getReplCoord()->waitUntilOpTime(&txn, ReadAfterOpTimeArgs(time));
 
         ASSERT_TRUE(result.didWait());
         ASSERT_OK(result.getStatus());
@@ -2073,8 +2068,7 @@ namespace {
         });
 
         auto result = getReplCoord()->waitUntilOpTime(&txn,
-                ReadAfterOpTimeArgs(OpTimeWithTermZero(100, 0),
-                                    Milliseconds(0)));
+                ReadAfterOpTimeArgs(OpTimeWithTermZero(100, 0)));
         pseudoLogOp.get();
 
         ASSERT_TRUE(result.didWait());
@@ -2098,31 +2092,13 @@ namespace {
             getReplCoord()->setMyLastOptime(opTimeToWait);
         });
 
-        auto result = getReplCoord()->waitUntilOpTime(&txn,
-                ReadAfterOpTimeArgs(opTimeToWait, Milliseconds(0)));
+        auto result = getReplCoord()->waitUntilOpTime(&txn, ReadAfterOpTimeArgs(opTimeToWait));
         pseudoLogOp.get();
 
         ASSERT_TRUE(result.didWait());
         ASSERT_OK(result.getStatus());
     }
 
-    TEST_F(ReplCoordTest, ReadAfterOpTimeTimeoutNoMaxTimeMS) {
-        OperationContextNoop txn;
-        assertStartSuccess(
-                BSON("_id" << "mySet" <<
-                     "version" << 2 <<
-                     "members" << BSON_ARRAY(BSON("host" << "node1:12345" << "_id" << 0))),
-                HostAndPort("node1", 12345));
-
-        getReplCoord()->setMyLastOptime(OpTimeWithTermZero(100, 0));
-
-        auto result = getReplCoord()->waitUntilOpTime(&txn,
-                ReadAfterOpTimeArgs(OpTimeWithTermZero(200, 0), Milliseconds(10)));
-
-        ASSERT_TRUE(result.didWait());
-        ASSERT_EQUALS(ErrorCodes::ReadAfterOptimeTimeout, result.getStatus());
-    }
-
     // TODO(schwerin): Unit test election id updating
 
 }  // namespace