diff options
author | Benety Goh <benety@mongodb.com> | 2022-08-29 19:42:58 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-08-30 00:29:51 +0000 |
commit | 58796facf40c99ddf8bb537adf563dc43aa0a863 (patch) | |
tree | 90359823e669e426388261a9ca4a51a3d6ddf9bf | |
parent | 2f93a13731d6b24c4101eca2d4652db18349d504 (diff) | |
download | mongo-58796facf40c99ddf8bb537adf563dc43aa0a863.tar.gz |
SERVER-68477 createIndexes overwrites NaN expireAfterSeconds before starting index build
-rw-r--r-- | jstests/noPassthrough/ttl_expire_nan.js | 29 | ||||
-rw-r--r-- | jstests/noPassthrough/ttl_expire_nan_warning_on_startup.js | 12 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_key_validate.cpp | 20 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_key_validate_test.cpp | 9 |
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( |