SERVER-93112 Partially revert SERVER-81890 to allow building indexes with simple and non-simple collations on the same key (#25638)

GitOrigin-RevId: 8cdd40c017bca323d68e97f6fab06aaa4b0f60db
This commit is contained in:
Jordi Olivares Provencio
2024-08-02 14:12:05 +02:00
committed by MongoDB Bot
parent 410968f1d6
commit 36635c8c4b
5 changed files with 67 additions and 91 deletions

View File

@@ -16,6 +16,7 @@ selector:
- jstests/core/index/wildcard/**/*.js
# v:1 index does not support collation.
- jstests/core/collation.js
- jstests/core/index/index_creation_on_different_collations.js
- jstests/core/query/expr/expr_index_use.js
# Timeseries collections are clustered, which use v:2 indexes.
- jstests/core/timeseries/**/*.js

View File

@@ -219,6 +219,8 @@ last-continuous:
ticket: SERVER-92930
- test_file: jstests/core/query/plan_cache/elem_match_index_diff_types.js
ticket: SERVER-92806
- test_file: jstests/core/index/index_creation_on_different_collations.js
ticket: SERVER-93112
suites: null
last-lts:
all:
@@ -450,4 +452,6 @@ last-lts:
ticket: SERVER-92930
- test_file: jstests/core/query/plan_cache/elem_match_index_diff_types.js
ticket: SERVER-92806
- test_file: jstests/core/index/index_creation_on_different_collations.js
ticket: SERVER-93112
suites: null

View File

@@ -0,0 +1,52 @@
/**
* This test ensures that creating an index on a collection with non-simple collation always
* respects the collation option used. This is required for sharding since the sharding index has to
* use the simple collation regardless of the collection's default collation.
*
* @tags: [
* # We do not expect there to be an existing collection as it can conflict with the
* # createCollection call with a "collection already exists with different options" error.
* assumes_no_implicit_collection_creation_after_drop
* ]
*/
import {IndexCatalogHelpers} from "jstests/libs/index_catalog_helpers.js";
const coll = db.index_creation_on_different_collations;
coll.drop();
const nonSimpleCollationOptions = {
locale: "en_US",
strength: 2,
caseLevel: false
};
assert.commandWorked(db.createCollection(coll.getName(), {collation: nonSimpleCollationOptions}));
// Create an index that doesn't specify the collation option. This should use the default value.
assert.commandWorked(
db.runCommand({createIndexes: coll.getName(), indexes: [{key: {x: 1}, name: "x_1"}]}));
assert.commandWorked(db.runCommand({
createIndexes: coll.getName(),
indexes: [{key: {x: 1}, name: "x_1_1", collation: {locale: 'simple'}}]
}));
const indexesFound = coll.getIndexes();
assert.neq(null,
IndexCatalogHelpers.findByKeyPattern(indexesFound, {x: 1}, {locale: "simple"}),
"Failed to find simple collation index for {x: 1} / Found: " + tojson(indexesFound));
assert.neq(null,
IndexCatalogHelpers.findByKeyPattern(indexesFound, {x: 1}, {
locale: "en_US",
caseLevel: false,
caseFirst: "off",
strength: 2,
numericOrdering: false,
alternate: "non-ignorable",
maxVariable: "punct",
normalization: false,
backwards: false,
version: "57.1"
}),
"Failed to find complex collation index for {x: 1} / Found: " + tojson(indexesFound));

View File

@@ -1,74 +0,0 @@
/**
* Test that a collection cloned during an index build doesn't cause issues with concurrent index
* operations referring to the same index.
*/
import {configureFailPoint} from "jstests/libs/fail_point_util.js";
import {IndexBuildTest} from "jstests/noPassthrough/libs/index_build.js";
const dbName = 'test';
const collectionName = 'coll';
const times = [
ISODate('1970-01-01T00:00:00'),
ISODate('1970-01-01T00:00:07'),
];
let docs = [];
for (const m of ['a', 'A', 'b', 'B'])
for (const t of times)
docs.push({t, m});
// Start one-node repl-set.
const rst = new ReplSetTest({nodes: 1});
rst.startSet();
rst.initiate();
const primary = rst.getPrimary();
const primaryDB = primary.getDB(dbName);
// Add a secondary.
const secondary = rst.add({
rsConfig: {votes: 0, priority: 0},
setParameter: {numInitialSyncAttempts: 1, logComponentVerbosity: '{verbosity: 1}'},
});
// Make the secondary pause once it has scanned the source's oplog. This will cause the secondary to
// apply all oplog entries from now on.
const failPoint = configureFailPoint(secondary, 'initialSyncHangBeforeCopyingDatabases');
rst.reInitiate();
failPoint.wait();
// The secondary has established the replay oplog begin timestamp. Proceed to create a collection.
primaryDB.createCollection(collectionName, {
collation: {locale: 'en_US', strength: 2},
});
const primaryColl = primaryDB.getCollection(collectionName);
// This will create a createIndexes oplog entry.
primaryColl.createIndex({m: 1, t: 1}, {
collation: {locale: 'simple'},
});
// Insert some documents so that another createIndexes becomes a two-phase index build.
assert.commandWorked(primaryColl.insert(docs));
// We will now drop all indexes and recreate them. We will also prevent the primary from completing
// the index build. This will cause the secondary to detect an unfinished index build during the
// cloning phase.
primaryColl.dropIndexes();
IndexBuildTest.pauseIndexBuilds(primary);
const createIdx = IndexBuildTest.startIndexBuild(primary, primaryColl.getFullName(), {m: 1, t: 1}, {
collation: {locale: 'simple'},
});
IndexBuildTest.waitForIndexBuildToStart(primaryDB, primaryColl.getName(), "m_1_t_1");
// Continue cloning, this will make the unfinished index build be processed by the CollectionCloner
// on the secondary. Pause the secondary before performing oplog replay.
const oplogApplierFp = configureFailPoint(secondary, 'initialSyncHangAfterDataCloning');
failPoint.off();
oplogApplierFp.wait();
// Complete the index build.
IndexBuildTest.resumeIndexBuilds(primary);
oplogApplierFp.off();
// There are now an unfinished index build + createIndex oplog entry on the secondary. Both entries
// refer to the same index, wait until it's caught up.
rst.awaitReplication();
createIdx();
rst.stopSet();

View File

@@ -511,18 +511,16 @@ StatusWith<BSONObj> IndexCatalogImpl::prepareSpecForCreate(
auto validatedSpec = swValidatedAndFixed.getValue();
// Normalise the spec
const auto normalSpec = IndexCatalog::normalizeIndexSpecs(opCtx, collection, validatedSpec);
// Check whether this is a non-_id index and there are any settings disallowing this server
// from building non-_id indexes.
Status status = _isNonIDIndexAndNotAllowedToBuild(opCtx, normalSpec);
Status status = _isNonIDIndexAndNotAllowedToBuild(opCtx, validatedSpec);
if (!status.isOK()) {
return status;
}
// First check against only the ready indexes for conflicts.
status = _doesSpecConflictWithExisting(opCtx, collection, normalSpec, InclusionPolicy::kReady);
status =
_doesSpecConflictWithExisting(opCtx, collection, validatedSpec, InclusionPolicy::kReady);
if (!status.isOK()) {
return status;
}
@@ -540,7 +538,7 @@ StatusWith<BSONObj> IndexCatalogImpl::prepareSpecForCreate(
// checking against all indexes occurred due to an in-progress index.
status = _doesSpecConflictWithExisting(opCtx,
collection,
normalSpec,
validatedSpec,
IndexCatalog::InclusionPolicy::kReady |
IndexCatalog::InclusionPolicy::kUnfinished |
IndexCatalog::InclusionPolicy::kFrozen);
@@ -572,14 +570,12 @@ std::vector<BSONObj> IndexCatalogImpl::removeExistingIndexesNoChecks(
// _doesSpecConflictWithExisting currently does more work than we require here: we are only
// interested in the index already exists error.
// Normalise the spec
const auto normalSpec = IndexCatalog::normalizeIndexSpecs(opCtx, collection, spec);
auto inclusionPolicy = removeInProgressIndexBuilds
? IndexCatalog::InclusionPolicy::kReady | IndexCatalog::InclusionPolicy::kUnfinished
: IndexCatalog::InclusionPolicy::kReady;
if (ErrorCodes::IndexAlreadyExists ==
_doesSpecConflictWithExisting(opCtx, collection, normalSpec, inclusionPolicy)) {
_doesSpecConflictWithExisting(opCtx, collection, spec, inclusionPolicy)) {
continue;
}
@@ -1103,11 +1099,9 @@ Status IndexCatalogImpl::_doesSpecConflictWithExisting(OperationContext* opCtx,
if (desc) {
// Index already exists with same name. Check whether the options are the same as well.
auto normalizedEntry = getEntry(desc)->getNormalizedEntry(opCtx, collection);
auto entry = getEntry(desc);
IndexDescriptor candidate(_getAccessMethodName(key), spec);
auto indexComparison =
candidate.compareIndexOptions(opCtx, collection->ns(), normalizedEntry.get());
auto indexComparison = candidate.compareIndexOptions(opCtx, collection->ns(), entry);
// Key pattern or another uniquely-identifying option differs. We can build this index,
// but not with the specified (duplicate) name. User must specify another index name.
@@ -1135,7 +1129,7 @@ Status IndexCatalogImpl::_doesSpecConflictWithExisting(OperationContext* opCtx,
// If an identical index exists, but it is frozen, return an error with a different
// error code to the user, forcing the user to drop before recreating the index.
if (normalizedEntry->isFrozen()) {
if (entry->isFrozen()) {
return Status(ErrorCodes::CannotCreateIndex,
str::stream()
<< "An identical, unfinished index '" << name
@@ -1165,11 +1159,10 @@ Status IndexCatalogImpl::_doesSpecConflictWithExisting(OperationContext* opCtx,
// Index already exists with a different name. Check whether the options are identical.
// We will return an error in either case, but this check allows us to generate a more
// informative error message.
auto normalizedEntry = getEntry(desc)->getNormalizedEntry(opCtx, collection);
auto entry = getEntry(desc);
IndexDescriptor candidate(_getAccessMethodName(key), spec);
auto indexComparison =
candidate.compareIndexOptions(opCtx, collection->ns(), normalizedEntry.get());
auto indexComparison = candidate.compareIndexOptions(opCtx, collection->ns(), entry);
// The candidate's key and uniquely-identifying options are equivalent to an existing
// index, but some other options are not identical. Return a message to that effect.