diff options
| -rw-r--r-- | buildscripts/resmokeconfig/suites/clustered_collection_passthrough.yml | 2 | ||||
| -rw-r--r-- | jstests/core/collmod_convert_to_unique_violations_size_limit.js | 119 | ||||
| -rw-r--r-- | src/mongo/db/catalog/coll_mod.cpp | 6 | ||||
| -rw-r--r-- | src/mongo/db/catalog/coll_mod_index.cpp | 36 | ||||
| -rw-r--r-- | src/mongo/db/catalog/coll_mod_index.h | 14 |
5 files changed, 153 insertions, 24 deletions
diff --git a/buildscripts/resmokeconfig/suites/clustered_collection_passthrough.yml b/buildscripts/resmokeconfig/suites/clustered_collection_passthrough.yml index 8cf08476490..77d160b3fb4 100644 --- a/buildscripts/resmokeconfig/suites/clustered_collection_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/clustered_collection_passthrough.yml @@ -63,6 +63,8 @@ selector: # TODO (SERVER-61259): $text not supported: "No query solutions" - jstests/core/fts6.js - jstests/core/fts_projection.js + # TODO (SERVER-64081): index keystring comparison not compatible + - jstests/core/collmod_convert_to_unique_violations_size_limit.js exclude_with_any_tags: - assumes_standalone_mongod diff --git a/jstests/core/collmod_convert_to_unique_violations_size_limit.js b/jstests/core/collmod_convert_to_unique_violations_size_limit.js new file mode 100644 index 00000000000..06b71afa354 --- /dev/null +++ b/jstests/core/collmod_convert_to_unique_violations_size_limit.js @@ -0,0 +1,119 @@ +/** + * Tests that the CannotConvertIndexToUnique error returned contains correct information about + * violations found when collMod fails to convert an index to unique and the size of the violations + * is large. + * + * @tags: [ + * # Cannot implicitly shard accessed collections because of collection existing when none + * # expected. + * assumes_no_implicit_collection_creation_after_drop, # common tag in collMod tests. + * requires_fcv_60, + * requires_non_retryable_commands, # common tag in collMod tests. + * # TODO(SERVER-61181): Fix validation errors under ephemeralForTest. + * incompatible_with_eft, + * # TODO(SERVER-61182): Fix WiredTigerKVEngine::alterIdentMetadata() under inMemory. + * requires_persistence, + * assumes_unsharded_collection, + * tenant_migration_incompatible, + * ] + */ +(function() { +'use strict'; + +const collModIndexUniqueEnabled = assert + .commandWorked(db.getMongo().adminCommand( + {getParameter: 1, featureFlagCollModIndexUnique: 1})) + .featureFlagCollModIndexUnique.value; + +if (!collModIndexUniqueEnabled) { + jsTestLog('Skipping test because the collMod unique index feature flag is disabled.'); + return; +} + +function sortViolationsArray(arr) { + // Sorting unsorted arrays of unsorted arrays -- Sort subarrays, then sort main array by first + // key of subarray. + for (let i = 0; i < arr.length; i++) { + arr[i].ids = arr[i].ids.sort(); + } + return arr.sort(function(a, b) { + if (a.ids[0] < b.ids[0]) { + return -1; + } + if (a.ids[0] > b.ids[0]) { + return 1; + } + return 0; + }); +} + +// Checks that the violations match what we expect. +function assertFailedWithViolations(result, expectedViolations, sizeLimitViolation) { + assert.commandFailedWithCode(result, ErrorCodes.CannotConvertIndexToUnique); + if (sizeLimitViolation) { + assert.eq( + result.errmsg, + "Cannot convert the index to unique. Too many conflicting documents were detected. " + + "Please resolve them and rerun collMod."); + } else { + assert.eq( + result.errmsg, + "Cannot convert the index to unique. Please resolve conflicting documents before " + + "running collMod again."); + } + assert.eq(bsonWoCompare(sortViolationsArray(result.violations), + sortViolationsArray(expectedViolations)), + 0, + "result violations " + result.violations + " does not match expected violations " + + expectedViolations); +} + +const collName = 'collmod_convert_to_unique_violations_size_limit'; +const coll = db.getCollection(collName); +coll.drop(); +assert.commandWorked(db.createCollection(collName)); + +// Creates regular indexes and try to use collMod to convert them to unique indexes. +assert.commandWorked(coll.createIndex({a: 1})); + +// Inserts 8 duplicate documents with 1MB large _id to exceed the 8MB size limit. +let ids = []; +let id; +const nDocs = 8; +for (let i = 0; i < nDocs; ++i) { + id = "x".repeat(1024 * 1024) + i; + assert.commandWorked(coll.insert({_id: id, a: 1})); + + // Only first 7 violations will be reported because 8th violation is over 8MB limit. + if (i < nDocs - 1) { + ids[i] = id; + } +} + +// Sets 'prepareUnique' before converting the index to unique. +assert.commandWorked( + db.runCommand({collMod: collName, index: {keyPattern: {a: 1}, prepareUnique: true}})); + +// Expects dryRun: true and unique: true conversion to fail with size exceeding violation. +assertFailedWithViolations( + db.runCommand({collMod: collName, index: {keyPattern: {a: 1}, unique: true}, dryRun: true}), + [{ids}], + true); + +// Expects unique: true conversion to fail with size exceeding violation. +assertFailedWithViolations( + db.runCommand({collMod: collName, index: {keyPattern: {a: 1}, unique: true}}), [{ids}], true); + +// Removes last violation. +assert.commandWorked(coll.deleteOne({_id: id})); + +// Expects dryRun: true and unique: true conversion to fail without size exceeding violation. +assertFailedWithViolations( + db.runCommand({collMod: collName, index: {keyPattern: {a: 1}, unique: true}, dryRun: true}), + [{ids}], + false); + +// Expects unique: true conversion to fail without size exceeding violation. +assertFailedWithViolations( + db.runCommand({collMod: collName, index: {keyPattern: {a: 1}, unique: true}}), [{ids}], false); +})(); diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index 12a7715d33d..52bc185ff29 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -564,8 +564,7 @@ Status _processCollModDryRunMode(OperationContext* opCtx, // Throws exception if index contains duplicates. auto violatingRecordsList = scanIndexForDuplicates(opCtx, collection, cmr.indexRequest.idx); if (!violatingRecordsList.empty()) { - uassertStatusOK(buildConvertUniqueErrorStatus( - buildDuplicateViolations(opCtx, collection, violatingRecordsList))); + uassertStatusOK(buildConvertUniqueErrorStatus(opCtx, collection, violatingRecordsList)); } return Status::OK(); @@ -603,8 +602,7 @@ StatusWith<const IndexDescriptor*> _setUpCollModIndexUnique(OperationContext* op nss); if (!violatingRecordsList.empty()) { - uassertStatusOK(buildConvertUniqueErrorStatus( - buildDuplicateViolations(opCtx, collection, violatingRecordsList))); + uassertStatusOK(buildConvertUniqueErrorStatus(opCtx, collection, violatingRecordsList)); } return idx; diff --git a/src/mongo/db/catalog/coll_mod_index.cpp b/src/mongo/db/catalog/coll_mod_index.cpp index 6bcb123e3aa..74d5e096979 100644 --- a/src/mongo/db/catalog/coll_mod_index.cpp +++ b/src/mongo/db/catalog/coll_mod_index.cpp @@ -145,8 +145,7 @@ void _processCollModIndexRequestUnique(OperationContext* opCtx, if (mode && *mode == repl::OplogApplication::Mode::kApplyOpsCmd) { auto duplicateRecordsList = scanIndexForDuplicates(opCtx, collection, idx); if (!duplicateRecordsList.empty()) { - uassertStatusOK(buildConvertUniqueErrorStatus( - buildDuplicateViolations(opCtx, collection, duplicateRecordsList))); + uassertStatusOK(buildConvertUniqueErrorStatus(opCtx, collection, duplicateRecordsList)); } } @@ -340,23 +339,38 @@ std::list<std::set<RecordId>> scanIndexForDuplicates( return duplicateRecordsList; } -BSONArray buildDuplicateViolations(OperationContext* opCtx, - const CollectionPtr& collection, - const std::list<std::set<RecordId>>& duplicateRecordsList) { +Status buildConvertUniqueErrorStatus(OperationContext* opCtx, + const CollectionPtr& collection, + const std::list<std::set<RecordId>>& duplicateRecordsList) { BSONArrayBuilder duplicateViolations; + size_t violationsSize = 0; + for (const auto& duplicateRecords : duplicateRecordsList) { BSONArrayBuilder currViolatingIds; for (const auto& recordId : duplicateRecords) { auto doc = collection->docFor(opCtx, recordId).value(); - currViolatingIds.append(doc["_id"]); + auto id = doc["_id"]; + violationsSize += id.size(); + + // Returns duplicate violations up to 8MB. + if (violationsSize > BSONObjMaxUserSize / 2) { + // Returns at least one violation. + if (duplicateViolations.arrSize() == 0 && currViolatingIds.arrSize() == 0) { + currViolatingIds.append(id); + } + if (currViolatingIds.arrSize() > 0) { + duplicateViolations.append(BSON("ids" << currViolatingIds.arr())); + } + return Status( + CannotConvertIndexToUniqueInfo(duplicateViolations.arr()), + "Cannot convert the index to unique. Too many conflicting documents were " + "detected. Please resolve them and rerun collMod."); + } + currViolatingIds.append(id); } duplicateViolations.append(BSON("ids" << currViolatingIds.arr())); } - return duplicateViolations.arr(); -} - -Status buildConvertUniqueErrorStatus(const BSONArray& violations) { - return Status(CannotConvertIndexToUniqueInfo(violations), + return Status(CannotConvertIndexToUniqueInfo(duplicateViolations.arr()), "Cannot convert the index to unique. Please resolve conflicting documents " "before running collMod again."); } diff --git a/src/mongo/db/catalog/coll_mod_index.h b/src/mongo/db/catalog/coll_mod_index.h index e5f78be7f55..acd7849d70e 100644 --- a/src/mongo/db/catalog/coll_mod_index.h +++ b/src/mongo/db/catalog/coll_mod_index.h @@ -79,15 +79,11 @@ std::list<std::set<RecordId>> scanIndexForDuplicates( boost::optional<KeyString::Value> firstKeyString = {}); /** - * Builds the BSONArray of the violations with duplicate index keys. + * Builds a BSONArray of the violations with duplicate index keys and returns the formatted error + * status for not being able to convert the index to unique. */ -BSONArray buildDuplicateViolations(OperationContext* opCtx, - const CollectionPtr& collection, - const std::list<std::set<RecordId>>& duplicateRecordsList); - -/** - * Returns the formatted error status for not being able to convert the index to unique. - */ -Status buildConvertUniqueErrorStatus(const BSONArray& violations); +Status buildConvertUniqueErrorStatus(OperationContext* opCtx, + const CollectionPtr& collection, + const std::list<std::set<RecordId>>& duplicateRecordsList); } // namespace mongo |