SERVER-68477 createIndexes overwrites NaN expireAfterSeconds before starting index build

(cherry picked from commit 58796facf40c99ddf8bb537adf563dc43aa0a863)

4 files changed, 68 insertions, 2 deletions

diff --git a/jstests/noPassthrough/ttl_expire_nan.js b/jstests/noPassthrough/ttl_expire_nan.js
index 6ab095192c4..8e07b9a1c2b 100644
--- a/jstests/noPassthrough/ttl_expire_nan.js
+++ b/jstests/noPassthrough/ttl_expire_nan.js
@@ -11,6 +11,7 @@
 (function() {
 'use strict';
 
+load("jstests/libs/fail_point_util.js");
 load('jstests/noPassthrough/libs/index_build.js');
 
 const rst = new ReplSetTest({
@@ -26,7 +27,16 @@ let primary = rst.getPrimary();
 const db = primary.getDB('test');
 const coll = db.t;
 
-assert.commandWorked(coll.createIndex({t: 1}, {expireAfterSeconds: NaN}));
+// The test cases here revolve around having a TTL index in the catalog with a NaN
+// 'expireAfterSeconds'. The current createIndexes behavior will overwrite NaN with int32::max
+// unless we use a fail point.
+const fp = configureFailPoint(primary, 'skipTTLIndexNaNExpireAfterSecondsValidation');
+try {
+    assert.commandWorked(coll.createIndex({t: 1}, {expireAfterSeconds: NaN}));
+} finally {
+    fp.off();
+}
+
 assert.commandWorked(coll.insert({_id: 0, t: ISODate()}));
 
 // Wait for "TTL indexes require the expire field to be numeric, skipping TTL job" log message.
@@ -89,5 +99,22 @@ assert.eq(collModOplogEntries.length,
           'TTL index with NaN expireAfterSeconds was not fixed using collMod during step-up: ' +
               tojson(rst.findOplog(primary, {op: {$ne: 'n'}}, /*limit=*/10).toArray()));
 
+// Confirm that createIndexes will overwrite a NaN 'expireAfterSeconds' in a TTL index before saving
+// it to the catalog and replicating it downstream.
+const coll2 = db.w;
+assert.commandWorked(coll2.createIndex({t: 1}, {expireAfterSeconds: NaN}));
+assert.commandWorked(coll2.insert({_id: 0, t: ISODate()}));
+
+// TTL index should be replicated to the secondary with a non-NaN 'expireAfterSeconds'.
+checkLog.containsJson(secondary, 20384, {
+    namespace: coll2.getFullName(),
+    properties: (spec) => {
+        jsTestLog('TTL index on secondary (with overwritten NaN expireAfterSeconds): ' +
+                  tojson(spec));
+        return spec.hasOwnProperty('expireAfterSeconds') && !isNaN(spec.expireAfterSeconds) &&
+            spec.expireAfterSeconds === newNodeSpec.expireAfterSeconds;
+    }
+});
+
 rst.stopSet();
 })();
diff --git a/jstests/noPassthrough/ttl_expire_nan_warning_on_startup.js b/jstests/noPassthrough/ttl_expire_nan_warning_on_startup.js
index d4b1d41f1fe..a285adc69a5 100644
--- a/jstests/noPassthrough/ttl_expire_nan_warning_on_startup.js
+++ b/jstests/noPassthrough/ttl_expire_nan_warning_on_startup.js
@@ -10,6 +10,7 @@
 (function() {
 'use strict';
 
+load("jstests/libs/fail_point_util.js");
 load('jstests/noPassthrough/libs/index_build.js');
 
 const rst = new ReplSetTest({nodes: [{}, {rsConfig: {votes: 0, priority: 0}}]});
@@ -20,7 +21,16 @@ let primary = rst.getPrimary();
 const db = primary.getDB('test');
 const coll = db.t;
 
-assert.commandWorked(coll.createIndex({t: 1}, {expireAfterSeconds: NaN}));
+// The test cases here revolve around having a TTL index in the catalog with a NaN
+// 'expireAfterSeconds'. The current createIndexes behavior will overwrite NaN with int32::max
+// unless we use a fail point.
+const fp = configureFailPoint(primary, 'skipTTLIndexNaNExpireAfterSecondsValidation');
+try {
+    assert.commandWorked(coll.createIndex({t: 1}, {expireAfterSeconds: NaN}));
+} finally {
+    fp.off();
+}
+
 assert.commandWorked(coll.insert({_id: 0, t: ISODate()}));
 
 // Force checkpoint in storage engine to ensure index is part of the catalog in
diff --git a/src/mongo/db/catalog/index_key_validate.cpp b/src/mongo/db/catalog/index_key_validate.cpp
index 67859cf3b6c..e814ceb3141 100644
--- a/src/mongo/db/catalog/index_key_validate.cpp
+++ b/src/mongo/db/catalog/index_key_validate.cpp
@@ -71,6 +71,10 @@ namespace {
 // specification.
 MONGO_FAIL_POINT_DEFINE(skipIndexCreateFieldNameValidation);
 
+// When the skipTTLIndexNaNExpireAfterSecondsValidation failpoint is enabled, validation for
+// TTL index 'expireAfterSeconds' will be disabled.
+MONGO_FAIL_POINT_DEFINE(skipTTLIndexNaNExpireAfterSecondsValidation);
+
 static const std::set<StringData> allowedIdIndexFieldNames = {
     IndexDescriptor::kCollationFieldName,
     IndexDescriptor::kIndexNameFieldName,
@@ -306,6 +310,7 @@ StatusWith<BSONObj> validateIndexSpec(OperationContext* opCtx, const BSONObj& in
     bool hasKeyPatternField = false;
     bool hasIndexNameField = false;
     bool hasNamespaceField = false;
+    bool isTTLIndexWithNaNExpireAfterSeconds = false;
     bool hasVersionField = false;
     bool hasCollationField = false;
     bool hasWeightsField = false;
@@ -564,6 +569,8 @@ StatusWith<BSONObj> validateIndexSpec(OperationContext* opCtx, const BSONObj& in
                     str::stream() << "The field '" << indexSpecElemFieldName
                                   << "' must be a number, but got "
                                   << typeName(indexSpecElem.type())};
+        } else if (IndexDescriptor::kExpireAfterSecondsFieldName == indexSpecElemFieldName) {
+            isTTLIndexWithNaNExpireAfterSeconds = indexSpecElem.isNaN();
         } else {
             // We can assume field name is valid at this point. Validation of fieldname is handled
             // prior to this in validateIndexSpecFieldNames().
@@ -635,6 +642,19 @@ StatusWith<BSONObj> validateIndexSpec(OperationContext* opCtx, const BSONObj& in
         modifiedSpec = modifiedSpec.removeField(IndexDescriptor::kNamespaceFieldName);
     }
 
+    if (isTTLIndexWithNaNExpireAfterSeconds &&
+        !skipTTLIndexNaNExpireAfterSecondsValidation.shouldFail()) {
+        // We create a new index specification with the 'expireAfterSeconds' field set as
+        // kExpireAfterSecondsForInactiveTTLIndex if the current value is NaN. A similar
+        // treatment is done in repairIndexSpec(). This rewrites the 'expireAfterSeconds'
+        // value to be compliant with the 'safeInt' IDL type for the listIndexes response.
+        BSONObjBuilder builder;
+        builder.appendNumber(IndexDescriptor::kExpireAfterSecondsFieldName,
+                             durationCount<Seconds>(kExpireAfterSecondsForInactiveTTLIndex));
+        auto obj = builder.obj();
+        modifiedSpec = modifiedSpec.addField(obj.firstElement());
+    }
+
     if (!hasVersionField) {
         // We create a new index specification with the 'v' field set as 'defaultIndexVersion' if
         // the field was omitted.
diff --git a/src/mongo/db/catalog/index_key_validate_test.cpp b/src/mongo/db/catalog/index_key_validate_test.cpp
index c60df1b34aa..34edafdc8fc 100644
--- a/src/mongo/db/catalog/index_key_validate_test.cpp
+++ b/src/mongo/db/catalog/index_key_validate_test.cpp
@@ -400,6 +400,15 @@ TEST(IndexKeyValidateTest, RemoveUnkownFieldsFromIndexSpecs) {
                    fromjson("{key: {a: 1}, name: 'index', safe: true, force: true}"))));
 }
 
+TEST(IndexKeyValidateTest, UpdateTTLIndexNaNExpireAfterSeconds) {
+    ASSERT(BSON("key" << BSON("a" << 1) << "name"
+                      << "index"
+                      << "expireAfterSeconds" << std::numeric_limits<int32_t>::max()
+                      << IndexDescriptor::kIndexVersionFieldName << IndexVersion::kV2)
+               .binaryEqual(unittest::assertGet(index_key_validate::validateIndexSpec(
+                   nullptr, fromjson("{key: {a: 1}, name: 'index', expireAfterSeconds: NaN}")))));
+}
+
 TEST(IndexKeyValidateTest, RepairIndexSpecs) {
     ASSERT(fromjson("{key: {a: 1}, name: 'index'}")
                .binaryEqual(index_key_validate::repairIndexSpec(