diff options
author | Arun Banala <arun.banala@10gen.com> | 2019-11-19 20:08:03 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2019-11-19 20:08:03 +0000 |
commit | 88956acd276472b1b4c0192f73b07d67c4cae29c (patch) | |
tree | c085c2f8a3e45d813cf79a94c7edc10d78ac0613 | |
parent | 9b26ebdb7013b2a5244261cda9c8d5e82ee3e80e (diff) | |
download | mongo-88956acd276472b1b4c0192f73b07d67c4cae29c.tar.gz |
SERVER-44571 Documents involved in SERVER-44050 corruption scenario cannot be updated or deleted after upgrade
25 files changed, 275 insertions, 18 deletions
diff --git a/etc/evergreen.yml b/etc/evergreen.yml index d6973ddc861..ba064d9d8c6 100644 --- a/etc/evergreen.yml +++ b/etc/evergreen.yml @@ -1480,7 +1480,7 @@ functions: --edition $edition \ --platform $platform \ --architecture $architecture \ - --useLatest 4.2 + --useLatest 4.2 4.2.1 "execute resmoke tests": &execute_resmoke_tests command: shell.exec diff --git a/jstests/multiVersion/hashed_index_bad_keys_cleanup.js b/jstests/multiVersion/hashed_index_bad_keys_cleanup.js new file mode 100644 index 00000000000..39675fdb730 --- /dev/null +++ b/jstests/multiVersion/hashed_index_bad_keys_cleanup.js @@ -0,0 +1,122 @@ +/** + * Prior to SERVER-44050, for hashed indexes, if there is an array along index field path, we did + * not fail insertion. We incorrectly stored empty index key value for those cases. This lead to + * corruption of index keys. + * + * In this test we verify that we are able to successfully update and delete documents that were + * involved in creating corrupt indexes. + * + * When we branch for 4.4, this test should be deleted in the master branch. In the v4.4 branch it + * should test a 4.2 => 4.4 upgrade scenario. + */ +(function() { +"use strict"; + +load("jstests/multiVersion/libs/multi_rs.js"); // For upgradeSet. +load("jstests/multiVersion/libs/verify_versions.js"); // For binVersion. + +const preBackportVersion = "4.2.1"; +const preBackportNodeOptions = { + binVersion: preBackportVersion +}; +const nodeOptionsOfLatestVersion = { + binVersion: "latest" +}; + +// Set up a new replSet consisting of 2 nodes, initially running on bad binaries. +const rst = new ReplSetTest({nodes: 2, nodeOptions: preBackportNodeOptions}); +rst.startSet(); +rst.initiate(); + +let testDB = rst.getPrimary().getDB(jsTestName()); +let coll = testDB.coll; +coll.drop(); + +// Verify that the replset is on binary version specified in 'preBackportVersion'. +assert.binVersion(testDB.getMongo(), preBackportVersion); + +// Insert bad documents using older version. +assert.commandWorked(coll.createIndex({"p.q.r": "hashed"})); +assert.commandWorked(coll.insert({_id: 1, p: []})); +assert.commandWorked(coll.insert({_id: 2, p: {q: [1]}})); +assert.commandWorked(coll.insert({_id: 3, p: [{q: 1}]})); +assert.commandWorked(coll.insert({_id: 4, a: 1, p: [{q: 1}]})); +assert.commandWorked(coll.insert({_id: 5, a: 1, p: [{q: 1}]})); + +// Helper function which runs validate() on primary and secondary nodes, then verifies that the +// command returned the expected result. +function assertValidateCmdReturned(expectedResult) { + const resFromPrimary = assert.commandWorked(coll.validate({full: true})); + assert.eq(resFromPrimary.valid, expectedResult, resFromPrimary); + + rst.awaitReplication(); + const testDBOnSecondary = rst.getSecondary().getDB(jsTestName()); + const resFromSecondary = assert.commandWorked(testDBOnSecondary.coll.validate({full: true})); + assert.eq(resFromSecondary.valid, expectedResult, resFromSecondary); +} + +// Confirm that validate() does not perceive a problem with the malformed documents. +assertValidateCmdReturned(true); + +// Upgrade the set to the new binary version. +rst.upgradeSet(nodeOptionsOfLatestVersion); +testDB = rst.getPrimary().getDB(jsTestName()); +coll = testDB.coll; + +// Verify that the five documents inserted earlier have their index keys. +let res = coll.find().hint({"p.q.r": "hashed"}).returnKey().itcount(); +assert.eq(res, 5); + +// Verify that after upgrade, inserting bad documents is not allowed. +const arrayAlongPathFailCode = 16766; +assert.commandFailedWithCode(coll.insert({p: []}), arrayAlongPathFailCode); +assert.commandFailedWithCode(coll.insert({p: [{q: 1}]}), arrayAlongPathFailCode); +assert.commandFailedWithCode(coll.insert({p: {q: {r: [3]}}}), arrayAlongPathFailCode); + +// After upgrade, validate() should now fail since there are existing bad documents. +assertValidateCmdReturned(false); + +// Deleting bad documents succeeds. +assert.commandWorked(coll.deleteOne({_id: 1})); +assert.commandWorked(coll.deleteMany({a: 1})); + +// Updating documents to contain array along field path should fail. +assert.commandFailedWithCode(coll.update({_id: 2}, {p: {q: [{r: 1}]}}), arrayAlongPathFailCode); +assert.commandFailedWithCode( + testDB.runCommand({findAndModify: coll.getName(), query: {_id: 2}, update: {p: {q: [{r: 1}]}}}), + arrayAlongPathFailCode); + +assert.commandFailedWithCode(coll.update({_id: 2}, {p: {q: {r: [3]}}}), arrayAlongPathFailCode); +assert.commandFailedWithCode(testDB.runCommand({ + update: coll.getName(), + updates: [ + {q: {_id: 3}, u: {$set: {p: {q: [{r: 1}]}}}}, + {q: {_id: 2}, u: {$set: {p: {q: [{r: 1}]}}}} + ], + ordered: false +}), + arrayAlongPathFailCode); + +// Verify that updating to a valid index field works. +assert.commandWorked(coll.update({_id: 2}, {p: {q: {r: 4}}})); + +// Verify that the index key is updated correctly by quering with hashed index. +res = coll.find({"p.q.r": 4}).hint({"p.q.r": "hashed"}).toArray(); +assert.eq(res, [{_id: 2, p: {q: {r: 4}}}]); + +// Validate should still fail since a bad document {_id: 3} exists. +assertValidateCmdReturned(false); + +// Delete the last remaining bad document. +assert.commandWorked(coll.deleteOne({_id: 3})); + +// Now that all the bad documents are deleted or updated, verify that validate succeeds. +assertValidateCmdReturned(true); + +// Verify that there is only one index key left (for {_id: 2}). +res = coll.find().hint({"p.q.r": "hashed"}).returnKey().itcount(); +assert.eq(res, 1); + +rst.awaitReplication(); +rst.stopSet(); +}());
\ No newline at end of file diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp index 958f286703e..fc123296941 100644 --- a/src/mongo/db/catalog/index_catalog_impl.cpp +++ b/src/mongo/db/catalog/index_catalog_impl.cpp @@ -1269,6 +1269,7 @@ Status IndexCatalogImpl::_indexFilteredRecords(OperationContext* opCtx, index->accessMethod()->getKeys(*bsonRecord.docPtr, options.getKeysMode, + IndexAccessMethod::GetKeysContext::kReadOrAddKeys, &keys, &multikeyMetadataKeys, &multikeyPaths, @@ -1422,6 +1423,7 @@ void IndexCatalogImpl::_unindexRecord(OperationContext* opCtx, entry->accessMethod()->getKeys(obj, IndexAccessMethod::GetKeysMode::kRelaxConstraintsUnfiltered, + IndexAccessMethod::GetKeysContext::kRemovingKeys, &keys, nullptr, nullptr, diff --git a/src/mongo/db/catalog/validate_adaptor.cpp b/src/mongo/db/catalog/validate_adaptor.cpp index 61e592abc25..75fa4f96a5c 100644 --- a/src/mongo/db/catalog/validate_adaptor.cpp +++ b/src/mongo/db/catalog/validate_adaptor.cpp @@ -98,6 +98,7 @@ Status ValidateAdaptor::validateRecord(OperationContext* opCtx, MultikeyPaths multikeyPaths; iam->getKeys(recordBson, IndexAccessMethod::GetKeysMode::kEnforceConstraints, + IndexAccessMethod::GetKeysContext::kReadOrAddKeys, &documentKeySet, &multikeyMetadataKeys, &multikeyPaths, diff --git a/src/mongo/db/exec/working_set_common.cpp b/src/mongo/db/exec/working_set_common.cpp index fa8198341fa..c8c9c3c00a5 100644 --- a/src/mongo/db/exec/working_set_common.cpp +++ b/src/mongo/db/exec/working_set_common.cpp @@ -80,6 +80,7 @@ bool WorkingSetCommon::fetch(OperationContext* opCtx, auto* iam = workingSet->retrieveIndexAccessMethod(memberKey.indexId); iam->getKeys(member->doc.value().toBson(), IndexAccessMethod::GetKeysMode::kEnforceConstraints, + IndexAccessMethod::GetKeysContext::kReadOrAddKeys, &keys, multikeyMetadataKeys, multikeyPaths, diff --git a/src/mongo/db/index/2d_access_method.cpp b/src/mongo/db/index/2d_access_method.cpp index 3aca45956ee..f8e59f52d69 100644 --- a/src/mongo/db/index/2d_access_method.cpp +++ b/src/mongo/db/index/2d_access_method.cpp @@ -51,6 +51,7 @@ TwoDAccessMethod::TwoDAccessMethod(IndexCatalogEntry* btreeState, /** Finds the key objects to put in an index */ void TwoDAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/2d_access_method.h b/src/mongo/db/index/2d_access_method.h index 5a8b426fb12..2c48b15ab14 100644 --- a/src/mongo/db/index/2d_access_method.h +++ b/src/mongo/db/index/2d_access_method.h @@ -59,6 +59,7 @@ private: * indexes don't support tracking path-level multikey information. */ void doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/btree_access_method.cpp b/src/mongo/db/index/btree_access_method.cpp index da13aa00f50..cf158940216 100644 --- a/src/mongo/db/index/btree_access_method.cpp +++ b/src/mongo/db/index/btree_access_method.cpp @@ -66,6 +66,7 @@ BtreeAccessMethod::BtreeAccessMethod(IndexCatalogEntry* btreeState, } void BtreeAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/btree_access_method.h b/src/mongo/db/index/btree_access_method.h index ca9db80f4c3..492cb791241 100644 --- a/src/mongo/db/index/btree_access_method.h +++ b/src/mongo/db/index/btree_access_method.h @@ -49,6 +49,7 @@ public: private: void doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/expression_keys_private.cpp b/src/mongo/db/index/expression_keys_private.cpp index eda7587c7dc..cf2f81a7020 100644 --- a/src/mongo/db/index/expression_keys_private.cpp +++ b/src/mongo/db/index/expression_keys_private.cpp @@ -49,6 +49,7 @@ #include "mongo/util/assert_util.h" #include "mongo/util/log.h" #include "mongo/util/str.h" + #include "third_party/s2/s2cell.h" #include "third_party/s2/s2regioncoverer.h" @@ -381,6 +382,7 @@ void ExpressionKeysPrivate::getHashKeys(const BSONObj& obj, KeyStringSet* keys, KeyString::Version keyStringVersion, Ordering ordering, + bool ignoreArraysAlongPath, boost::optional<RecordId> id) { static const BSONObj nullObj = BSON("" << BSONNULL); auto hasFieldValue = false; @@ -391,14 +393,27 @@ void ExpressionKeysPrivate::getHashKeys(const BSONObj& obj, auto fieldVal = dps::extractElementAtPathOrArrayAlongPath(obj, cstr); // If we hit an array while traversing the path, 'cstr' will point to the path component - // immediately following the array, or the null termination byte if the final field in the - // path was an array. + // immediately following the array, or the null termination byte if the terminal path + // component was an array. In the latter case, 'remainingPath' will be empty. + auto remainingPath = StringData(cstr); + + // If 'ignoreArraysAlongPath' is set, we want to use the behaviour prior to SERVER-44050, + // which is to allow arrays along the field path (except the terminal path). This is done so + // that the document keys inserted prior to SERVER-44050 can be deleted or updated after the + // upgrade, allowing users to recover from the possible index corruption. The old behaviour + // before SERVER-44050 was to store 'null' index key if we encountered an array along the + // index field path. We will use the same logic in the context of removing index keys. + if (ignoreArraysAlongPath && fieldVal.type() == BSONType::Array && !remainingPath.empty()) { + fieldVal = nullObj.firstElement(); + } + + // Otherwise, throw if an array was encountered at any point along the path. uassert(16766, str::stream() << "Error: hashed indexes do not currently support array values. " "Found array at path: " << indexPath.substr(0, - indexPath.size() - StringData(cstr).size() - - !StringData(cstr).empty()), + indexPath.size() - remainingPath.size() - + !remainingPath.empty()), fieldVal.type() != BSONType::Array); BSONObj fieldValObj; diff --git a/src/mongo/db/index/expression_keys_private.h b/src/mongo/db/index/expression_keys_private.h index 8a03b566d7d..a4cb319c041 100644 --- a/src/mongo/db/index/expression_keys_private.h +++ b/src/mongo/db/index/expression_keys_private.h @@ -94,6 +94,7 @@ public: KeyStringSet* keys, KeyString::Version keyStringVersion, Ordering ordering, + bool ignoreArraysAlongPath, boost::optional<RecordId> id = boost::none); /** diff --git a/src/mongo/db/index/fts_access_method.cpp b/src/mongo/db/index/fts_access_method.cpp index 0734d5fa5fd..29a45b43abd 100644 --- a/src/mongo/db/index/fts_access_method.cpp +++ b/src/mongo/db/index/fts_access_method.cpp @@ -40,6 +40,7 @@ FTSAccessMethod::FTSAccessMethod(IndexCatalogEntry* btreeState, _ftsSpec(btreeState->descriptor()->infoObj()) {} void FTSAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/fts_access_method.h b/src/mongo/db/index/fts_access_method.h index 016246b0208..1897cb9b503 100644 --- a/src/mongo/db/index/fts_access_method.h +++ b/src/mongo/db/index/fts_access_method.h @@ -53,6 +53,7 @@ private: * indexes don't support tracking path-level multikey information. */ void doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/hash_access_method.cpp b/src/mongo/db/index/hash_access_method.cpp index 0a218177016..a12c980b6da 100644 --- a/src/mongo/db/index/hash_access_method.cpp +++ b/src/mongo/db/index/hash_access_method.cpp @@ -50,6 +50,7 @@ HashAccessMethod::HashAccessMethod(IndexCatalogEntry* btreeState, } void HashAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, @@ -63,6 +64,7 @@ void HashAccessMethod::doGetKeys(const BSONObj& obj, keys, getSortedDataInterface()->getKeyStringVersion(), getSortedDataInterface()->getOrdering(), + (context == GetKeysContext::kRemovingKeys), id); } diff --git a/src/mongo/db/index/hash_access_method.h b/src/mongo/db/index/hash_access_method.h index 9bdaef9c0b7..ed539e90f7b 100644 --- a/src/mongo/db/index/hash_access_method.h +++ b/src/mongo/db/index/hash_access_method.h @@ -56,6 +56,7 @@ private: * indexes don't support tracking path-level multikey information. */ void doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/hash_key_generator_test.cpp b/src/mongo/db/index/hash_key_generator_test.cpp index c97879cb204..f2babba8cb4 100644 --- a/src/mongo/db/index/hash_key_generator_test.cpp +++ b/src/mongo/db/index/hash_key_generator_test.cpp @@ -98,7 +98,8 @@ TEST(HashKeyGeneratorTest, CollationAppliedBeforeHashing) { &collator, &actualKeys, KeyString::Version::kLatestVersion, - Ordering::make(BSONObj())); + Ordering::make(BSONObj()), + false); BSONObj backwardsObj = fromjson("{a: 'gnirts'}"); KeyStringSet expectedKeys; @@ -121,7 +122,8 @@ TEST(HashKeyGeneratorTest, CollationDoesNotAffectNonStringFields) { &collator, &actualKeys, KeyString::Version::kLatestVersion, - Ordering::make(BSONObj())); + Ordering::make(BSONObj()), + false); KeyStringSet expectedKeys; expectedKeys.insert(makeHashKey(obj["a"])); @@ -144,7 +146,8 @@ TEST(HashKeyGeneratorTest, CollatorAppliedBeforeHashingNestedObject) { &collator, &actualKeys, KeyString::Version::kLatestVersion, - Ordering::make(BSONObj())); + Ordering::make(BSONObj()), + false); KeyStringSet expectedKeys; expectedKeys.insert(makeHashKey(backwardsObj["a"])); @@ -167,7 +170,8 @@ TEST(HashKeyGeneratorTest, CollationAppliedforAllIndexFields) { &collator, &actualKeys, KeyString::Version::kLatestVersion, - Ordering::make(BSONObj())); + Ordering::make(BSONObj()), + false); KeyStringSet expectedKeys; KeyString::HeapBuilder keyString(KeyString::Version::kLatestVersion, Ordering::make(BSONObj())); @@ -189,7 +193,8 @@ TEST(HashKeyGeneratorTest, NoCollation) { nullptr, &actualKeys, KeyString::Version::kLatestVersion, - Ordering::make(BSONObj())); + Ordering::make(BSONObj()), + false); KeyStringSet expectedKeys; expectedKeys.insert(makeHashKey(obj["a"])); @@ -209,7 +214,8 @@ TEST(HashKeyGeneratorTest, CompoundIndexEmptyObject) { nullptr, &actualKeys, KeyString::Version::kLatestVersion, - Ordering::make(BSONObj())); + Ordering::make(BSONObj()), + false); // Verify that we inserted null indexes for empty input object. KeyStringSet expectedKeys; @@ -235,7 +241,8 @@ TEST(HashKeyGeneratorTest, SparseIndex) { nullptr, &actualKeys, KeyString::Version::kLatestVersion, - Ordering::make(BSONObj())); + Ordering::make(BSONObj()), + false); // Verify that no index entries were added to the sparse index. ASSERT(assertKeysetsEqual(KeyStringSet(), actualKeys)); } @@ -252,7 +259,8 @@ TEST(HashKeyGeneratorTest, SparseIndexWithAFieldPresent) { nullptr, &actualKeys, KeyString::Version::kLatestVersion, - Ordering::make(BSONObj())); + Ordering::make(BSONObj()), + false); // Verify that we inserted null entries for the misssing fields. KeyStringSet expectedKeys; @@ -266,4 +274,63 @@ TEST(HashKeyGeneratorTest, SparseIndexWithAFieldPresent) { ASSERT(assertKeysetsEqual(expectedKeys, actualKeys)); } +TEST(HashKeyGeneratorTest, ArrayAlongIndexFieldPathFails) { + BSONObj obj = fromjson("{a: []}"); + KeyStringSet actualKeys; + BSONObj indexSpec = fromjson("{'a.b.c': 'hashed'}"); + ASSERT_THROWS_CODE(ExpressionKeysPrivate::getHashKeys(obj, + indexSpec, + kHashSeed, + kHashVersion, + false, + nullptr, + &actualKeys, + KeyString::Version::kLatestVersion, + Ordering::make(BSONObj()), + false), + DBException, + 16766); +} + +TEST(HashKeyGeneratorTest, ArrayAlongIndexFieldPathDoesNotFailWhenIgnoreFlagIsSet) { + BSONObj obj = fromjson("{a: []}"); + KeyStringSet actualKeys; + BSONObj indexSpec = fromjson("{'a.b.c': 'hashed'}"); + ExpressionKeysPrivate::getHashKeys(obj, + indexSpec, + kHashSeed, + kHashVersion, + false, + nullptr, + &actualKeys, + KeyString::Version::kLatestVersion, + Ordering::make(BSONObj()), + true // ignoreArraysAlongPath + ); + + KeyStringSet expectedKeys; + expectedKeys.insert(makeHashKey(BSON("" << BSONNULL).firstElement())); + ASSERT(assertKeysetsEqual(expectedKeys, actualKeys)); +} + +TEST(HashKeyGeneratorTest, ArrayAtTerminalPathAlwaysFails) { + BSONObj obj = fromjson("{a : {b: {c: [1]}}}"); + KeyStringSet actualKeys; + BSONObj indexSpec = fromjson("{'a.b.c': 'hashed'}"); + ASSERT_THROWS_CODE(ExpressionKeysPrivate::getHashKeys(obj, + indexSpec, + kHashSeed, + kHashVersion, + true, // isSparse + nullptr, + &actualKeys, + KeyString::Version::kLatestVersion, + Ordering::make(BSONObj()), + true, // ignoreArraysAlongPath + boost::none), + DBException, + 16766); +} + + } // namespace diff --git a/src/mongo/db/index/haystack_access_method.cpp b/src/mongo/db/index/haystack_access_method.cpp index bd4993ff3e5..e309dcaebed 100644 --- a/src/mongo/db/index/haystack_access_method.cpp +++ b/src/mongo/db/index/haystack_access_method.cpp @@ -65,6 +65,7 @@ HaystackAccessMethod::HaystackAccessMethod(IndexCatalogEntry* btreeState, } void HaystackAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/haystack_access_method.h b/src/mongo/db/index/haystack_access_method.h index d434fd9ca38..8056a6df3e2 100644 --- a/src/mongo/db/index/haystack_access_method.h +++ b/src/mongo/db/index/haystack_access_method.h @@ -77,6 +77,7 @@ private: * geoHaystack indexes don't support tracking path-level multikey information. */ void doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/index_access_method.cpp b/src/mongo/db/index/index_access_method.cpp index 321c682a5ec..ccc6ce3c2f2 100644 --- a/src/mongo/db/index/index_access_method.cpp +++ b/src/mongo/db/index/index_access_method.cpp @@ -131,7 +131,13 @@ Status AbstractIndexAccessMethod::insert(OperationContext* opCtx, MultikeyPaths multikeyPaths; // Delegate to the subclass. - getKeys(obj, options.getKeysMode, &keys, &multikeyMetadataKeys, &multikeyPaths, loc); + getKeys(obj, + options.getKeysMode, + GetKeysContext::kReadOrAddKeys, + &keys, + &multikeyMetadataKeys, + &multikeyPaths, + loc); return insertKeys(opCtx, {keys.begin(), keys.end()}, @@ -240,6 +246,7 @@ RecordId AbstractIndexAccessMethod::findSingle(OperationContext* opCtx, MultikeyPaths* multikeyPaths = nullptr; getKeys(requestedKey, GetKeysMode::kEnforceConstraints, + GetKeysContext::kReadOrAddKeys, &keys, multikeyMetadataKeys, multikeyPaths); @@ -351,12 +358,19 @@ void AbstractIndexAccessMethod::prepareUpdate(OperationContext* opCtx, // There's no need to compute the prefixes of the indexed fields that possibly caused the // index to be multikey when the old version of the document was written since the index // metadata isn't updated when keys are deleted. - getKeys(from, getKeysMode, &ticket->oldKeys, nullptr, nullptr, record); + getKeys(from, + getKeysMode, + GetKeysContext::kRemovingKeys, + &ticket->oldKeys, + nullptr, + nullptr, + record); } if (!indexFilter || indexFilter->matchesBSON(to)) { getKeys(to, options.getKeysMode, + GetKeysContext::kReadOrAddKeys, &ticket->newKeys, &ticket->newMultikeyMetadataKeys, &ticket->newMultikeyPaths, @@ -490,8 +504,13 @@ Status AbstractIndexAccessMethod::BulkBuilderImpl::insert(OperationContext* opCt MultikeyPaths multikeyPaths; try { - _real->getKeys( - obj, options.getKeysMode, &keys, &_multikeyMetadataKeys, &multikeyPaths, loc); + _real->getKeys(obj, + options.getKeysMode, + GetKeysContext::kReadOrAddKeys, + &keys, + &_multikeyMetadataKeys, + &multikeyPaths, + loc); } catch (...) { return exceptionToStatus(); } @@ -646,6 +665,7 @@ void AbstractIndexAccessMethod::setIndexIsMultikey(OperationContext* opCtx, Mult void AbstractIndexAccessMethod::getKeys(const BSONObj& obj, GetKeysMode mode, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, @@ -674,7 +694,7 @@ void AbstractIndexAccessMethod::getKeys(const BSONObj& obj, 13026, 13027}; try { - doGetKeys(obj, keys, multikeyMetadataKeys, multikeyPaths, id); + doGetKeys(obj, context, keys, multikeyMetadataKeys, multikeyPaths, id); } catch (const AssertionException& ex) { // Suppress all indexing errors when mode is kRelaxConstraints. if (mode == GetKeysMode::kEnforceConstraints) { diff --git a/src/mongo/db/index/index_access_method.h b/src/mongo/db/index/index_access_method.h index 0961321b269..d18a0b74577 100644 --- a/src/mongo/db/index/index_access_method.h +++ b/src/mongo/db/index/index_access_method.h @@ -276,6 +276,12 @@ public: }; /** + * Specifies whether getKeys is being used in the context of creating new keys or deleting + * existing keys. + */ + enum class GetKeysContext { kRemovingKeys, kReadOrAddKeys }; + + /** * Fills 'keys' with the keys that should be generated for 'obj' on this index. * Based on 'mode', it will honor or ignore index constraints, e.g. duplicated key, key too * long, and geo index parsing errors. The ignoring of constraints is for replication due to @@ -294,6 +300,7 @@ public: */ virtual void getKeys(const BSONObj& obj, GetKeysMode mode, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, @@ -499,6 +506,7 @@ public: void getKeys(const BSONObj& obj, GetKeysMode mode, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, @@ -526,6 +534,7 @@ protected: * information that must be stored in a reserved keyspace within the index. */ virtual void doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/s2_access_method.cpp b/src/mongo/db/index/s2_access_method.cpp index 777415dc1a5..da5bc634ee9 100644 --- a/src/mongo/db/index/s2_access_method.cpp +++ b/src/mongo/db/index/s2_access_method.cpp @@ -125,6 +125,7 @@ StatusWith<BSONObj> S2AccessMethod::fixSpec(const BSONObj& specObj) { } void S2AccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/s2_access_method.h b/src/mongo/db/index/s2_access_method.h index e8ff0bd2cb1..d3ed49d918e 100644 --- a/src/mongo/db/index/s2_access_method.h +++ b/src/mongo/db/index/s2_access_method.h @@ -65,6 +65,7 @@ private: * be multikey as a result of inserting 'keys'. */ void doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/wildcard_access_method.cpp b/src/mongo/db/index/wildcard_access_method.cpp index f77fe1d84a4..ec1626aa75f 100644 --- a/src/mongo/db/index/wildcard_access_method.cpp +++ b/src/mongo/db/index/wildcard_access_method.cpp @@ -54,6 +54,7 @@ bool WildcardAccessMethod::shouldMarkIndexAsMultikey( } void WildcardAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/db/index/wildcard_access_method.h b/src/mongo/db/index/wildcard_access_method.h index 09a3f5dc530..da03e69c9dc 100644 --- a/src/mongo/db/index/wildcard_access_method.h +++ b/src/mongo/db/index/wildcard_access_method.h @@ -94,6 +94,7 @@ public: private: void doGetKeys(const BSONObj& obj, + GetKeysContext context, KeyStringSet* keys, KeyStringSet* multikeyMetadataKeys, MultikeyPaths* multikeyPaths, diff --git a/src/mongo/dbtests/validate_tests.cpp b/src/mongo/dbtests/validate_tests.cpp index fb7b739f3b6..1b0a862f92b 100644 --- a/src/mongo/dbtests/validate_tests.cpp +++ b/src/mongo/dbtests/validate_tests.cpp @@ -891,6 +891,7 @@ public: KeyStringSet keys; iam->getKeys(actualKey, IndexAccessMethod::GetKeysMode::kRelaxConstraintsUnfiltered, + IndexAccessMethod::GetKeysContext::kReadOrAddKeys, &keys, nullptr, nullptr, @@ -1287,6 +1288,7 @@ public: KeyStringSet keys; iam->getKeys(actualKey, IndexAccessMethod::GetKeysMode::kRelaxConstraintsUnfiltered, + IndexAccessMethod::GetKeysContext::kReadOrAddKeys, &keys, nullptr, nullptr, @@ -1486,6 +1488,7 @@ public: KeyStringSet keys; iam->getKeys(actualKey, IndexAccessMethod::GetKeysMode::kRelaxConstraintsUnfiltered, + IndexAccessMethod::GetKeysContext::kReadOrAddKeys, &keys, nullptr, nullptr, @@ -1520,6 +1523,7 @@ public: KeyStringSet keys; iam->getKeys(actualKey, IndexAccessMethod::GetKeysMode::kRelaxConstraintsUnfiltered, + IndexAccessMethod::GetKeysContext::kReadOrAddKeys, &keys, nullptr, nullptr, |