SERVER-26927 support integer values for maxStalenessSeconds

5 files changed, 88 insertions, 135 deletions

diff --git a/src/mongo/client/read_preference.cpp b/src/mongo/client/read_preference.cpp
index ea16f3365ad..e54a85357c5 100644
--- a/src/mongo/client/read_preference.cpp
+++ b/src/mongo/client/read_preference.cpp
@@ -54,9 +54,6 @@ const char kSecondaryOnly[] = "secondary";
 const char kSecondaryPreferred[] = "secondaryPreferred";
 const char kNearest[] = "nearest";
 
-// Avoid overflow errors when converting from seconds to milliseconds
-const auto kMaximalMaxStalenessSecondsValue(durationCount<Seconds>(Milliseconds::max()));
-
 StringData readPreferenceName(ReadPreference pref) {
     switch (pref) {
         case ReadPreference::PrimaryOnly:
@@ -120,7 +117,7 @@ TagSet defaultTagSetForMode(ReadPreference mode) {
 /**
  * Replica set refresh period on the task executor.
  */
-const Milliseconds ReadPreferenceSetting::kMinimalMaxStalenessValue(60000);
+const Seconds ReadPreferenceSetting::kMinimalMaxStalenessValue(90);
 
 TagSet::TagSet() : _tags(BSON_ARRAY(BSONObj())) {}
 
@@ -130,11 +127,13 @@ TagSet TagSet::primaryOnly() {
 
 ReadPreferenceSetting::ReadPreferenceSetting(ReadPreference pref,
                                              TagSet tags,
-                                             Milliseconds maxStalenessMS)
-    : pref(std::move(pref)), tags(std::move(tags)), maxStalenessMS(std::move(maxStalenessMS)) {}
+                                             Seconds maxStalenessSeconds)
+    : pref(std::move(pref)),
+      tags(std::move(tags)),
+      maxStalenessSeconds(std::move(maxStalenessSeconds)) {}
 
-ReadPreferenceSetting::ReadPreferenceSetting(ReadPreference pref, Milliseconds maxStalenessMS)
-    : ReadPreferenceSetting(pref, defaultTagSetForMode(pref), maxStalenessMS) {}
+ReadPreferenceSetting::ReadPreferenceSetting(ReadPreference pref, Seconds maxStalenessSeconds)
+    : ReadPreferenceSetting(pref, defaultTagSetForMode(pref), maxStalenessSeconds) {}
 
 ReadPreferenceSetting::ReadPreferenceSetting(ReadPreference pref, TagSet tags)
     : pref(std::move(pref)), tags(std::move(tags)) {}
@@ -185,56 +184,40 @@ StatusWith<ReadPreferenceSetting> ReadPreferenceSetting::fromBSON(const BSONObj&
         return tagExtractStatus;
     }
 
-    double maxStalenessSecondsValue;
-
-    Status maxStalenessSecondsExtractStatus = bsonExtractDoubleField(
-        readPrefObj, kMaxStalenessSecondsFieldName, &maxStalenessSecondsValue);
+    long long maxStalenessSecondsValue;
+    auto maxStalenessSecondsExtractStatus = bsonExtractIntegerFieldWithDefault(
+        readPrefObj, kMaxStalenessSecondsFieldName, 0, &maxStalenessSecondsValue);
 
-    if (maxStalenessSecondsExtractStatus == ErrorCodes::NoSuchKey) {
-        return ReadPreferenceSetting(mode, tags);
-    } else if (!maxStalenessSecondsExtractStatus.isOK()) {
+    if (!maxStalenessSecondsExtractStatus.isOK()) {
         return maxStalenessSecondsExtractStatus;
     }
 
-    if (maxStalenessSecondsValue < 0.0) {
-        return {ErrorCodes::BadValue,
-                str::stream() << kMaxStalenessSecondsFieldName << " value can not be negative"};
-    }
-
-    if (maxStalenessSecondsValue > kMaximalMaxStalenessSecondsValue) {
-        return {ErrorCodes::MaxStalenessOutOfRange,
-                str::stream() << kMaxStalenessSecondsFieldName << " value can not exceed "
-                              << kMaximalMaxStalenessSecondsValue};
+    if (maxStalenessSecondsValue && maxStalenessSecondsValue < 0) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << kMaxStalenessSecondsFieldName
+                                    << " must be a non-negative integer");
     }
 
-    if (maxStalenessSecondsValue == 0.0) {
-        return ReadPreferenceSetting(mode, tags);
+    if (maxStalenessSecondsValue && maxStalenessSecondsValue >= Seconds::max().count()) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << kMaxStalenessSecondsFieldName << " value can not exceed "
+                                    << Seconds::max().count());
     }
 
-    // Use a lambda to do the double seconds to integer milliseconds conversion in order to
-    // encapsulate the usage of helper variables
-    const Milliseconds requestedMaxStalenessMS = [maxStalenessSecondsValue] {
-        double integerPart;
-        const double fractionalPart = std::modf(maxStalenessSecondsValue, &integerPart);
-
-        return Seconds(static_cast<long long>(integerPart)) +
-            Milliseconds(static_cast<long long>(fractionalPart *
-                                                durationCount<Milliseconds>(Seconds(1))));
-    }();
-
-    if (requestedMaxStalenessMS < kMinimalMaxStalenessValue) {
-        return {ErrorCodes::MaxStalenessOutOfRange,
-                str::stream() << kMaxStalenessSecondsFieldName << " value can not be less than "
-                              << kMinimalMaxStalenessValue};
+    if (maxStalenessSecondsValue && maxStalenessSecondsValue < kMinimalMaxStalenessValue.count()) {
+        return Status(ErrorCodes::MaxStalenessOutOfRange,
+                      str::stream() << kMaxStalenessSecondsFieldName
+                                    << " value can not be less than "
+                                    << kMinimalMaxStalenessValue.count());
     }
 
-    if (mode == ReadPreference::PrimaryOnly) {
-        return {ErrorCodes::BadValue,
-                str::stream() << kMaxStalenessSecondsFieldName
-                              << " can not be set for the primary mode"};
+    if ((mode == ReadPreference::PrimaryOnly) && maxStalenessSecondsValue) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << kMaxStalenessSecondsFieldName
+                                    << " can not be set for the primary mode");
     }
 
-    return ReadPreferenceSetting(mode, tags, requestedMaxStalenessMS);
+    return ReadPreferenceSetting(mode, tags, Seconds(maxStalenessSecondsValue));
 }
 
 BSONObj ReadPreferenceSetting::toBSON() const {
@@ -243,10 +226,8 @@ BSONObj ReadPreferenceSetting::toBSON() const {
     if (tags != defaultTagSetForMode(pref)) {
         bob.append(kTagsFieldName, tags.getTagBSON());
     }
-    if (maxStalenessMS.count() > 0) {
-        bob.append(kMaxStalenessSecondsFieldName,
-                   static_cast<double>(maxStalenessMS.count()) /
-                       durationCount<Milliseconds>(Seconds(1)));
+    if (maxStalenessSeconds.count() > 0) {
+        bob.append(kMaxStalenessSecondsFieldName, maxStalenessSeconds.count());
     }
     return bob.obj();
 }
diff --git a/src/mongo/client/read_preference.h b/src/mongo/client/read_preference.h
index 037a895555e..0e80e872970 100644
--- a/src/mongo/client/read_preference.h
+++ b/src/mongo/client/read_preference.h
@@ -122,14 +122,14 @@ struct ReadPreferenceSetting {
      *     object's copy of tag will have the iterator in the initial
      *     position).
      */
-    ReadPreferenceSetting(ReadPreference pref, TagSet tags, Milliseconds maxStalenessMS);
-    ReadPreferenceSetting(ReadPreference pref, Milliseconds maxStalenessMS);
+    ReadPreferenceSetting(ReadPreference pref, TagSet tags, Seconds maxStalenessSeconds);
+    ReadPreferenceSetting(ReadPreference pref, Seconds maxStalenessSeconds);
     ReadPreferenceSetting(ReadPreference pref, TagSet tags);
     explicit ReadPreferenceSetting(ReadPreference pref);
 
     inline bool equals(const ReadPreferenceSetting& other) const {
         return (pref == other.pref) && (tags == other.tags) &&
-            (maxStalenessMS == other.maxStalenessMS) && (minOpTime == other.minOpTime);
+            (maxStalenessSeconds == other.maxStalenessSeconds) && (minOpTime == other.minOpTime);
     }
 
     /**
@@ -144,24 +144,23 @@ struct ReadPreferenceSetting {
 
     /**
      * Parses a ReadPreferenceSetting from a BSON document of the form:
-     * { mode: <mode>, tags: <array of tags>, maxStalenessMS: Number }. The 'mode' element must a
-     * string equal to either
-     * "primary", "primaryPreferred", "secondary", "secondaryPreferred", or "nearest". Although
-     * the tags array is intended to be an array of unique BSON documents, no further validation
-     * is performed on it other than checking that it is an array, and that it is empty if
-     * 'mode' is 'primary'.
+     * { mode: <mode>, tags: <array of tags>, maxStalenessSeconds: Number }. The 'mode' element must
+     * be a string equal to either "primary", "primaryPreferred", "secondary", "secondaryPreferred",
+     * or "nearest". Although the tags array is intended to be an array of unique BSON documents, no
+     * further validation is performed on it other than checking that it is an array, and that it is
+     * empty if 'mode' is 'primary'.
      */
     static StatusWith<ReadPreferenceSetting> fromBSON(const BSONObj& readPrefSettingObj);
 
     ReadPreference pref;
     TagSet tags;
-    Milliseconds maxStalenessMS{};
+    Seconds maxStalenessSeconds{};
     repl::OpTime minOpTime{};
 
     /**
-     * The minimal value maxStalenessMS can have. It MUST be ReplicaSetMonitor::kRefreshPeriod * 2
+     * The minimal value maxStalenessSeconds can have.
      */
-    static const Milliseconds kMinimalMaxStalenessValue;
+    static const Seconds kMinimalMaxStalenessValue;
 };
 
 }  // namespace mongo
diff --git a/src/mongo/client/read_preference_test.cpp b/src/mongo/client/read_preference_test.cpp
index e080d059d08..dcf20107433 100644
--- a/src/mongo/client/read_preference_test.cpp
+++ b/src/mongo/client/read_preference_test.cpp
@@ -32,20 +32,17 @@
 #include "mongo/unittest/unittest.h"
 #include "mongo/util/duration.h"
 
-namespace mongo {
 namespace {
 
-using unittest::assertGet;
+using namespace mongo;
 
-const Seconds minMaxStalenessSeconds(
-    durationCount<Seconds>(ReadPreferenceSetting::kMinimalMaxStalenessValue));
+static const Seconds kMinMaxStaleness = ReadPreferenceSetting::kMinimalMaxStalenessValue;
 
 void checkParse(const BSONObj& rpsObj, const ReadPreferenceSetting& expected) {
-    const auto rps = assertGet(ReadPreferenceSetting::fromBSON(rpsObj));
-    if (!rps.equals(expected)) {
-        FAIL(str::stream() << "Expected " << expected.toString() << " does not match actual "
-                           << rps.toString());
-    }
+    const auto swRps = ReadPreferenceSetting::fromBSON(rpsObj);
+    ASSERT_OK(swRps.getStatus());
+    const auto rps = swRps.getValue();
+    ASSERT_TRUE(rps.equals(expected));
 }
 
 TEST(ReadPreferenceSetting, ParseValid) {
@@ -72,26 +69,14 @@ TEST(ReadPreferenceSetting, ParseValid) {
     checkParse(BSON("mode"
                     << "secondary"
                     << "maxStalenessSeconds"
-                    << minMaxStalenessSeconds.count()),
-               ReadPreferenceSetting(ReadPreference::SecondaryOnly, minMaxStalenessSeconds));
+                    << kMinMaxStaleness.count()),
+               ReadPreferenceSetting(ReadPreference::SecondaryOnly, kMinMaxStaleness));
 
     checkParse(BSON("mode"
                     << "secondary"
                     << "maxStalenessSeconds"
                     << 0),
-               ReadPreferenceSetting(ReadPreference::SecondaryOnly, Milliseconds(0)));
-
-    checkParse(BSON("mode"
-                    << "secondary"
-                    << "maxStalenessSeconds"
-                    << 61LL),
-               ReadPreferenceSetting(ReadPreference::SecondaryOnly, Milliseconds(61000)));
-
-    checkParse(BSON("mode"
-                    << "secondary"
-                    << "maxStalenessSeconds"
-                    << 63.46),
-               ReadPreferenceSetting(ReadPreference::SecondaryOnly, Milliseconds(63460)));
+               ReadPreferenceSetting(ReadPreference::SecondaryOnly, Seconds(0)));
 
     checkParse(BSON("mode"
                     << "secondary"
@@ -99,11 +84,11 @@ TEST(ReadPreferenceSetting, ParseValid) {
                     << BSON_ARRAY(BSON("dc"
                                        << "ny"))
                     << "maxStalenessSeconds"
-                    << minMaxStalenessSeconds.count()),
+                    << kMinMaxStaleness.count()),
                ReadPreferenceSetting(ReadPreference::SecondaryOnly,
                                      TagSet(BSON_ARRAY(BSON("dc"
                                                             << "ny"))),
-                                     minMaxStalenessSeconds));
+                                     kMinMaxStaleness));
 }
 
 void checkParseFails(const BSONObj& rpsObj) {
@@ -122,14 +107,14 @@ TEST(ReadPreferenceSetting, NonEquality) {
                                          << "ca")
                                     << BSON("foo"
                                             << "bar")));
-    auto rps = ReadPreferenceSetting(ReadPreference::Nearest, tagSet, minMaxStalenessSeconds);
+    auto rps = ReadPreferenceSetting(ReadPreference::Nearest, tagSet, kMinMaxStaleness);
 
-    auto unexpected1 = ReadPreferenceSetting(
-        ReadPreference::Nearest, TagSet::primaryOnly(), minMaxStalenessSeconds);
+    auto unexpected1 =
+        ReadPreferenceSetting(ReadPreference::Nearest, TagSet::primaryOnly(), kMinMaxStaleness);
     ASSERT_FALSE(rps.equals(unexpected1));
 
     auto unexpected2 = ReadPreferenceSetting(
-        ReadPreference::Nearest, tagSet, Seconds(minMaxStalenessSeconds.count() + 1));
+        ReadPreference::Nearest, tagSet, Seconds(kMinMaxStaleness.count() + 1));
     ASSERT_FALSE(rps.equals(unexpected2));
 }
 
@@ -155,43 +140,41 @@ TEST(ReadPreferenceSetting, ParseInvalid) {
                          << "bad"));
 
     // maxStalenessSeconds is negative
-    checkParseFailsWithError(BSON("mode"
-                                  << "secondary"
-                                  << "maxStalenessSeconds"
-                                  << -1),
-                             ErrorCodes::BadValue);
+    checkParseFails(BSON("mode"
+                         << "secondary"
+                         << "maxStalenessSeconds"
+                         << -1));
 
     // maxStalenessSeconds is NaN
-    checkParseFailsWithError(BSON("mode"
-                                  << "secondary"
-                                  << "maxStalenessSeconds"
-                                  << "ONE"),
-                             ErrorCodes::TypeMismatch);
+    checkParseFails(BSON("mode"
+                         << "secondary"
+                         << "maxStalenessSeconds"
+                         << "ONE"));
 
     // maxStalenessSeconds and primary
     checkParseFails(BSON("mode"
                          << "primary"
                          << "maxStalenessSeconds"
-                         << minMaxStalenessSeconds.count()));
+                         << kMinMaxStaleness.count()));
 
     // maxStalenessSeconds is less than min
     checkParseFailsWithError(BSON("mode"
                                   << "primary"
                                   << "maxStalenessSeconds"
-                                  << minMaxStalenessSeconds.count() - 1),
+                                  << Seconds(kMinMaxStaleness.count() - 1).count()),
                              ErrorCodes::MaxStalenessOutOfRange);
 
-    // maxStalenessSeconds is greater than max type value for milliseconds
-    checkParseFailsWithError(BSON("mode"
-                                  << "secondary"
-                                  << "maxStalenessSeconds"
-                                  << Milliseconds::max().count()),
-                             ErrorCodes::MaxStalenessOutOfRange);
+    // maxStalenessSeconds is greater than max
+    checkParseFails(BSON("mode"
+                         << "secondary"
+                         << "maxStalenessSeconds"
+                         << Seconds::max().count()));
 }
 
 void checkRoundtrip(const ReadPreferenceSetting& rps) {
-    auto parsed = assertGet(ReadPreferenceSetting::fromBSON(rps.toBSON()));
-    ASSERT_TRUE(parsed.equals(rps));
+    auto parsed = ReadPreferenceSetting::fromBSON(rps.toBSON());
+    ASSERT_OK(parsed.getStatus());
+    ASSERT_TRUE(parsed.getValue().equals(rps));
 }
 
 TEST(ReadPreferenceSetting, Roundtrip) {
@@ -215,15 +198,7 @@ TEST(ReadPreferenceSetting, Roundtrip) {
                                                                 << "ca")
                                                            << BSON("foo"
                                                                    << "bar"))),
-                                         minMaxStalenessSeconds));
-
-    checkRoundtrip(ReadPreferenceSetting(ReadPreference::Nearest,
-                                         TagSet(BSON_ARRAY(BSON("dc"
-                                                                << "ca")
-                                                           << BSON("foo"
-                                                                   << "bar"))),
-                                         Milliseconds(63246)));
+                                         kMinMaxStaleness));
 }
 
 }  // namespace
-}  // namespace mongo
diff --git a/src/mongo/client/replica_set_monitor.cpp b/src/mongo/client/replica_set_monitor.cpp
index 9c1a503a5e1..2d58216994e 100644
--- a/src/mongo/client/replica_set_monitor.cpp
+++ b/src/mongo/client/replica_set_monitor.cpp
@@ -165,7 +165,6 @@ struct HostNotIn {
 
 /**
  * Replica set refresh period on the task executor.
- * This value must be equal to ReadPreferenceSettings::kMinimalMaxStalenessValue / 2
  */
 const Seconds kRefreshPeriod(30);
 }  // namespace
@@ -186,7 +185,6 @@ ReplicaSetMonitor::ReplicaSetMonitor(const MongoURI& uri)
 void ReplicaSetMonitor::init() {
     stdx::lock_guard<stdx::mutex> lk(_mutex);
     invariant(_executor);
-    invariant(kRefreshPeriod * 2 == ReadPreferenceSetting::kMinimalMaxStalenessValue);
     std::weak_ptr<ReplicaSetMonitor> that(shared_from_this());
     auto status = _executor->scheduleWork([=](const CallbackArgs& cbArgs) {
         if (auto ptr = that.lock()) {
@@ -1030,12 +1028,12 @@ HostAndPort SetState::getMatchingHost(const ReadPreferenceSetting& criteria) con
             if (!out.empty())
                 return out;
             return getMatchingHost(ReadPreferenceSetting(
-                ReadPreference::SecondaryOnly, criteria.tags, criteria.maxStalenessMS));
+                ReadPreference::SecondaryOnly, criteria.tags, criteria.maxStalenessSeconds));
         }
 
         case ReadPreference::SecondaryPreferred: {
             HostAndPort out = getMatchingHost(ReadPreferenceSetting(
-                ReadPreference::SecondaryOnly, criteria.tags, criteria.maxStalenessMS));
+                ReadPreference::SecondaryOnly, criteria.tags, criteria.maxStalenessSeconds));
             if (!out.empty())
                 return out;
             // NOTE: the spec says we should use the primary even if tags don't match
@@ -1058,7 +1056,7 @@ HostAndPort SetState::getMatchingHost(const ReadPreferenceSetting& criteria) con
                 return true;
             };
             // build comparator
-            if (criteria.maxStalenessMS.count()) {
+            if (criteria.maxStalenessSeconds.count()) {
                 auto masterIt = std::find_if(nodes.begin(), nodes.end(), isMaster);
                 if (masterIt == nodes.end() || !masterIt->lastWriteDate.toMillisSinceEpoch()) {
                     auto writeDateCmp = [](const Node* a, const Node* b) -> bool {
@@ -1078,18 +1076,19 @@ HostAndPort SetState::getMatchingHost(const ReadPreferenceSetting& criteria) con
                     } else {
                         Date_t maxWriteTime = (*latestSecNode)->lastWriteDate;
                         matchNode = [=](const Node& node) -> bool {
-                            return Milliseconds(maxWriteTime - node.lastWriteDate) +
+                            return duration_cast<Seconds>(maxWriteTime - node.lastWriteDate) +
                                 kRefreshPeriod <=
-                                criteria.maxStalenessMS;
+                                criteria.maxStalenessSeconds;
                         };
                     }
                 } else {
-                    Milliseconds primaryStaleness =
-                        masterIt->lastWriteDateUpdateTime - masterIt->lastWriteDate;
+                    Seconds primaryStaleness = duration_cast<Seconds>(
+                        masterIt->lastWriteDateUpdateTime - masterIt->lastWriteDate);
                     matchNode = [=](const Node& node) -> bool {
-                        return Milliseconds(node.lastWriteDateUpdateTime - node.lastWriteDate) -
+                        return duration_cast<Seconds>(node.lastWriteDateUpdateTime -
+                                                      node.lastWriteDate) -
                             primaryStaleness + kRefreshPeriod <=
-                            criteria.maxStalenessMS;
+                            criteria.maxStalenessSeconds;
                     };
                 }
             }
diff --git a/src/mongo/client/replica_set_monitor_test.cpp b/src/mongo/client/replica_set_monitor_test.cpp
index 403e03a985a..838728c5946 100644
--- a/src/mongo/client/replica_set_monitor_test.cpp
+++ b/src/mongo/client/replica_set_monitor_test.cpp
@@ -1333,8 +1333,7 @@ TEST(ReplicaSetMonitor, MaxStalenessMSMatch) {
     Refresher refresher(state);
     repl::OpTime opTime{Timestamp{10, 10}, 10};
 
-    const ReadPreferenceSetting secondary(
-        ReadPreference::SecondaryOnly, TagSet(), Milliseconds(100000));
+    const ReadPreferenceSetting secondary(ReadPreference::SecondaryOnly, TagSet(), Seconds(100));
     BSONArray hosts = BSON_ARRAY("a"
                                  << "b"
                                  << "c");
@@ -1739,7 +1738,7 @@ TEST(ReplicaSetMonitor, MaxStalenessMSZeroNoLastWrite) {
     SetStatePtr state = std::make_shared<SetState>("name", basicSeedsSet);
     Refresher refresher(state);
 
-    const ReadPreferenceSetting secondary(ReadPreference::SecondaryOnly, TagSet(), Milliseconds(0));
+    const ReadPreferenceSetting secondary(ReadPreference::SecondaryOnly, TagSet(), Seconds(0));
     BSONArray hosts = BSON_ARRAY("a"
                                  << "b"
                                  << "c");