diff options
| -rw-r--r-- | jstests/core/timeseries/timeseries_delete_hint.js | 206 | ||||
| -rw-r--r-- | jstests/libs/parallelTester.js | 3 | ||||
| -rw-r--r-- | src/mongo/db/commands/write_commands.cpp | 57 | ||||
| -rw-r--r-- | src/mongo/db/timeseries/timeseries_update_delete_util.cpp | 36 | ||||
| -rw-r--r-- | src/mongo/db/timeseries/timeseries_update_delete_util.h | 11 |
5 files changed, 268 insertions, 45 deletions
diff --git a/jstests/core/timeseries/timeseries_delete_hint.js b/jstests/core/timeseries/timeseries_delete_hint.js new file mode 100644 index 00000000000..043a96c4e01 --- /dev/null +++ b/jstests/core/timeseries/timeseries_delete_hint.js @@ -0,0 +1,206 @@ +/** + * Tests running the delete command with a hint on a time-series collection. + * @tags: [ + * assumes_no_implicit_collection_creation_after_drop, + * does_not_support_stepdowns, + * does_not_support_transactions, + * requires_fcv_50, + * requires_getmore, + * # $currentOp can't run with a readConcern other than 'local'. + * assumes_read_concern_unchanged, + * # This test only synchronizes deletes on the primary. + * assumes_read_preference_unchanged, + * # Fail points in this test do not exist on mongos. + * assumes_against_mongod_not_mongos, + * uses_parallel_shell, + * ] + */ +(function() { +"use strict"; + +load("jstests/core/timeseries/libs/timeseries.js"); +load("jstests/libs/curop_helpers.js"); +load('jstests/libs/parallel_shell_helpers.js'); + +if (!TimeseriesTest.timeseriesUpdatesAndDeletesEnabled(db.getMongo())) { + jsTestLog("Skipping test because the time-series updates and deletes feature flag is disabled"); + return; +} + +const timeFieldName = "time"; +const metaFieldName = "tag"; +const collName = 't'; +const bucketsCollNamespace = jsTestName() + '.system.buckets.' + collName; + +const validateDeleteIndex = (docsToInsert, + expectedRemainingDocs, + expectedNRemoved, + deleteQuery, + indexes, + expectedPlan, + {expectedErrorCode = null} = {}) => { + const testDB = db.getSiblingDB(jsTestName()); + const awaitTestDelete = startParallelShell(funWithArgs( + function(docsToInsert, + expectedRemainingDocs, + expectedNRemoved, + deleteQuery, + indexes, + timeFieldName, + metaFieldName, + collName, + expectedErrorCode) { + const testDB = db.getSiblingDB(jsTestName()); + const coll = testDB.getCollection(collName); + + assert.commandWorked(testDB.createCollection( + coll.getName(), + {timeseries: {timeField: timeFieldName, metaField: metaFieldName}})); + assert.commandWorked(coll.createIndexes(indexes)); + + assert.commandWorked(coll.insert(docsToInsert)); + + assert.commandWorked(testDB.adminCommand( + {configureFailPoint: "hangBeforeChildRemoveOpFinishes", mode: "alwaysOn"})); + const res = expectedErrorCode + ? assert.commandFailedWithCode( + testDB.runCommand({delete: coll.getName(), deletes: deleteQuery}), + expectedErrorCode) + : assert.commandWorked( + testDB.runCommand({delete: coll.getName(), deletes: deleteQuery})); + assert.eq(res["n"], expectedNRemoved); + assert.docEq(coll.find({}, {_id: 0}).toArray(), expectedRemainingDocs); + assert(coll.drop()); + }, + docsToInsert, + expectedRemainingDocs, + expectedNRemoved, + deleteQuery, + indexes, + timeFieldName, + metaFieldName, + collName, + expectedErrorCode)); + + for (let childCount = 0; childCount < deleteQuery.length; ++childCount) { + const childCurOp = waitForCurOpByFailPoint( + testDB, bucketsCollNamespace, "hangBeforeChildRemoveOpFinishes")[0]; + + // Assert the plan uses the expected index. + if (!expectedErrorCode) { + assert.eq(childCurOp['planSummary'], expectedPlan); + } + + assert.commandWorked(testDB.adminCommand( + {configureFailPoint: "hangBeforeChildRemoveOpIsPopped", mode: "alwaysOn"})); + assert.commandWorked(testDB.adminCommand( + {configureFailPoint: "hangBeforeChildRemoveOpFinishes", mode: "off"})); + + waitForCurOpByFailPoint(testDB, bucketsCollNamespace, "hangBeforeChildRemoveOpIsPopped"); + + // Enable the hangBeforeChildRemoveOpFinishes fail point if this is not the last child. + if (childCount + 1 < deleteQuery.length) { + assert.commandWorked(testDB.adminCommand( + {configureFailPoint: "hangBeforeChildRemoveOpFinishes", mode: "alwaysOn"})); + } + + assert.commandWorked(testDB.adminCommand( + {configureFailPoint: "hangBeforeChildRemoveOpIsPopped", mode: "off"})); + } + + // Wait for testDelete to finish. + awaitTestDelete(); +}; + +const objA = { + [timeFieldName]: ISODate(), + "measurement": {"A": "cpu"}, + [metaFieldName]: {a: "A"} +}; +const objB = { + [timeFieldName]: ISODate(), + "measurement": {"A": "cpu"}, + [metaFieldName]: {b: "B"} +}; +const objC = { + [timeFieldName]: ISODate(), + "measurement": {"A": "cpu"}, + [metaFieldName]: {c: "C"} +}; + +// Query using the metaField index as a hint. +validateDeleteIndex([objA, objB, objC], + [objB, objC], + 1, + [{q: {[metaFieldName]: {a: "A"}}, limit: 0, hint: {[metaFieldName]: 1}}], + [{[metaFieldName]: 1}], + "IXSCAN { meta: 1 }"); + +// Query on a collection with a compound index using the timeField and metaField indexes as +// hints. +validateDeleteIndex( + [objA, objB, objC], + [objB, objC], + 1, + [{q: {[metaFieldName]: {a: "A"}}, limit: 0, hint: {[timeFieldName]: 1, [metaFieldName]: 1}}], + [{[timeFieldName]: 1, [metaFieldName]: 1}], + "IXSCAN { control.min.time: 1, control.max.time: 1, meta: 1 }"); + +// Query on a collection with a compound index using the timeField and metaField indexes as +// hints +validateDeleteIndex( + [objA, objB, objC], + [objA, objC], + 1, + [{q: {[metaFieldName]: {b: "B"}}, limit: 0, hint: {[timeFieldName]: -1, [metaFieldName]: 1}}], + [{[timeFieldName]: -1, [metaFieldName]: 1}], + "IXSCAN { control.max.time: -1, control.min.time: -1, meta: 1 }"); + +// Query on a collection with a compound index using the timeField and metaField index names +// as hints. +validateDeleteIndex([objA, objB, objC], + [objA, objC], + 1, + [{ + q: {[metaFieldName]: {b: "B"}}, + limit: 0, + hint: timeFieldName + "_1_" + metaFieldName + "_1" + }], + [{[timeFieldName]: 1, [metaFieldName]: 1}], + "IXSCAN { control.min.time: 1, control.max.time: 1, meta: 1 }"); + +// Query on a collection with multiple indexes using the timeField index as a hint. +validateDeleteIndex([objA, objB, objC], + [objA, objB], + 1, + [{q: {[metaFieldName]: {c: "C"}}, limit: 0, hint: {[timeFieldName]: 1}}], + [{[metaFieldName]: -1}, {[timeFieldName]: 1}], + "IXSCAN { control.min.time: 1, control.max.time: 1 }"); + +// Query on a collection with multiple indexes using the timeField index as a hint. +validateDeleteIndex([objA, objB, objC], + [objA, objB], + 1, + [{q: {[metaFieldName]: {c: "C"}}, limit: 0, hint: {[timeFieldName]: 1}}], + [{[metaFieldName]: -1}, {[timeFieldName]: 1}], + "IXSCAN { control.min.time: 1, control.max.time: 1 }"); + +// Query on a collection with multiple indexes using an invalid index name. +validateDeleteIndex([objA, objB, objC], + [objA, objB, objC], + 0, + [{q: {[metaFieldName]: {c: "C"}}, limit: 0, hint: "test_hint"}], + [{[metaFieldName]: -1}, {[timeFieldName]: 1}], + "IXSCAN { control.min.time: 1, control.max.time: 1 }", + {expectedErrorCode: ErrorCodes.BadValue}); + +// TODO: SERVER-58519 Uncomment this test. +// // Query on a collection with multiple indexes using an invalid index spec. +// validateDeleteIndex([objA, objB, objC], +// [objA, objB, objC], +// 0, +// [{q: {[metaFieldName]: {c: "C"}}, limit: 0, hint: {"test_hint": 1}}], +// [{[metaFieldName]: -1}, {[timeFieldName]: 1}], +// "IXSCAN { control.min.time: 1, control.max.time: 1 }", +// {expectedErrorCode: ErrorCodes.BadValue}); +})(); diff --git a/jstests/libs/parallelTester.js b/jstests/libs/parallelTester.js index fb4edc625eb..d1146548c74 100644 --- a/jstests/libs/parallelTester.js +++ b/jstests/libs/parallelTester.js @@ -228,6 +228,9 @@ if (typeof _threadInject != "undefined") { "bench_test1.js", "bench_test2.js", "bench_test3.js", + // This test causes delete commands to hang, which may affect other tests running + // concurrently. + "timeseries/timeseries_delete_hint.js" ]); // Get files, including files in subdirectories. diff --git a/src/mongo/db/commands/write_commands.cpp b/src/mongo/db/commands/write_commands.cpp index d86302032dd..f81e905254b 100644 --- a/src/mongo/db/commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands.cpp @@ -63,6 +63,7 @@ #include "mongo/db/storage/duplicate_key_error_info.h" #include "mongo/db/storage/storage_parameters_gen.h" #include "mongo/db/timeseries/bucket_catalog.h" +#include "mongo/db/timeseries/timeseries_index_schema_conversion_functions.h" #include "mongo/db/timeseries/timeseries_update_delete_util.h" #include "mongo/db/transaction_participant.h" #include "mongo/db/views/view_catalog.h" @@ -129,48 +130,6 @@ bool isMetaFieldFirstElementOfDottedPathField(StringData metaField, StringData f return field.substr(0, field.find('.')) == metaField; } -// TODO: SERVER-58394 Remove this method and combine its logic with -// timeseries::replaceTimeseriesQueryMetaFieldName(). -/** - * Recurses through the mutablebson element query and replaces any occurrences of the - * metaField with "meta" accounting for queries that may be in dot notation. shouldReplaceFieldValue - * is set for $expr queries when "$" + the metaField should be subsitutited for "$meta". - */ -void replaceTimeseriesQueryMetaFieldName(mutablebson::Element elem, - const StringData& metaField, - bool shouldReplaceFieldValue = false) { - if (metaField.empty()) { - return; - } - shouldReplaceFieldValue = (elem.getFieldName() != "$literal") && - (shouldReplaceFieldValue || (elem.getFieldName() == "$expr")); - if (isMetaFieldFirstElementOfDottedPathField(metaField, elem.getFieldName())) { - size_t dotIndex = elem.getFieldName().find('.'); - dotIndex >= elem.getFieldName().size() - ? invariantStatusOK(elem.rename("meta")) - : invariantStatusOK(elem.rename( - "meta" + - elem.getFieldName().substr(dotIndex, elem.getFieldName().size() - dotIndex))); - } - // Substitute element fieldValue with "$meta" if element is a subField of $expr, not a subField - // of $literal, and the element fieldValue is "$" + the metaField. - else if (shouldReplaceFieldValue && elem.isType(BSONType::String) && - isMetaFieldFirstElementOfDottedPathField("$" + metaField, elem.getValueString())) { - size_t dotIndex = elem.getValueString().find('.'); - dotIndex >= elem.getValueString().size() - ? invariantStatusOK(elem.setValueString("$meta")) - : invariantStatusOK(elem.setValueString( - "$meta" + - elem.getValueString().substr(dotIndex, elem.getValueString().size() - dotIndex))); - } - if (elem.hasChildren()) { - for (size_t i = 0; i < elem.countChildren(); ++i) { - replaceTimeseriesQueryMetaFieldName( - elem.findNthChild(i), metaField, shouldReplaceFieldValue); - } - } -} - // Default for control.version in time-series bucket collection. const int kTimeseriesControlVersion = 1; @@ -1541,10 +1500,18 @@ public: timeseriesDeletes.reserve(request().getDeletes().size()); for (auto& deleteEntry : request().getDeletes()) { // TODO: SERVER-58394 Call timeseries::translateQuery() here instead. - mutablebson::Document doc(deleteEntry.getQ()); - replaceTimeseriesQueryMetaFieldName(doc.root(), metaField); + mutablebson::Document queryDoc(deleteEntry.getQ()); + timeseries::replaceTimeseriesQueryMetaFieldName(queryDoc.root(), metaField); timeseriesDeletes.push_back(deleteEntry); - timeseriesDeletes.back().setQ(doc.getObject()); + timeseriesDeletes.back().setQ(queryDoc.getObject()); + + // Only translate the hint if it is specified by index spec. + if (deleteEntry.getHint().firstElement().fieldNameStringData() != "$hint"_sd || + deleteEntry.getHint().firstElement().type() != BSONType::String) { + timeseriesDeletes.back().setHint( + uassertStatusOK(timeseries::createBucketsIndexSpecFromTimeseriesIndexSpec( + *timeseriesOptions, deleteEntry.getHint()))); + } } write_ops::DeleteCommandRequest timeseriesDeleteReq( diff --git a/src/mongo/db/timeseries/timeseries_update_delete_util.cpp b/src/mongo/db/timeseries/timeseries_update_delete_util.cpp index 5eafea0d762..f38a5681e19 100644 --- a/src/mongo/db/timeseries/timeseries_update_delete_util.cpp +++ b/src/mongo/db/timeseries/timeseries_update_delete_util.cpp @@ -31,6 +31,7 @@ #include "mongo/db/pipeline/expression_context.h" #include "mongo/db/pipeline/pipeline.h" +#include "mongo/db/timeseries/timeseries_constants.h" namespace mongo::timeseries { namespace { @@ -259,4 +260,39 @@ write_ops::UpdateOpEntry translateUpdate(const BSONObj& translatedQuery, } MONGO_UNREACHABLE; } + +void replaceTimeseriesQueryMetaFieldName(mutablebson::Element elem, + const StringData& metaField, + bool shouldReplaceFieldValue) { + if (metaField.empty()) { + return; + } + shouldReplaceFieldValue = (elem.getFieldName() != "$literal") && + (shouldReplaceFieldValue || (elem.getFieldName() == "$expr")); + if (isMetaFieldFirstElementOfDottedPathField(elem.getFieldName(), metaField)) { + size_t dotIndex = elem.getFieldName().find('.'); + dotIndex >= elem.getFieldName().size() + ? invariantStatusOK(elem.rename("meta")) + : invariantStatusOK(elem.rename( + "meta" + + elem.getFieldName().substr(dotIndex, elem.getFieldName().size() - dotIndex))); + } + // Substitute element fieldValue with "$meta" if element is a subField of $expr, not a subField + // of $literal, and the element fieldValue is "$" + the metaField. + else if (shouldReplaceFieldValue && elem.isType(BSONType::String) && + isMetaFieldFirstElementOfDottedPathField(elem.getValueString(), "$" + metaField)) { + size_t dotIndex = elem.getValueString().find('.'); + dotIndex >= elem.getValueString().size() + ? invariantStatusOK(elem.setValueString("$meta")) + : invariantStatusOK(elem.setValueString( + "$meta" + + elem.getValueString().substr(dotIndex, elem.getValueString().size() - dotIndex))); + } + if (elem.hasChildren()) { + for (size_t i = 0; i < elem.countChildren(); ++i) { + replaceTimeseriesQueryMetaFieldName( + elem.findNthChild(i), metaField, shouldReplaceFieldValue); + } + } +} } // namespace mongo::timeseries diff --git a/src/mongo/db/timeseries/timeseries_update_delete_util.h b/src/mongo/db/timeseries/timeseries_update_delete_util.h index 12cb1887ac3..43a7bf6850f 100644 --- a/src/mongo/db/timeseries/timeseries_update_delete_util.h +++ b/src/mongo/db/timeseries/timeseries_update_delete_util.h @@ -66,4 +66,15 @@ write_ops::UpdateOpEntry translateUpdate(const BSONObj& translatedQuery, const write_ops::UpdateModification& updateMod, StringData metaField); +// TODO: SERVER-58394 Remove this method and combine its logic with +// timeseries::replaceTimeseriesQueryMetaFieldName(). +/** + * Recurses through the mutablebson element query and replaces any occurrences of the + * metaField with "meta" accounting for queries that may be in dot notation. shouldReplaceFieldValue + * is set for $expr queries when "$" + the metaField should be substituted for "$meta". + */ +void replaceTimeseriesQueryMetaFieldName(mutablebson::Element elem, + const StringData& metaField, + bool shouldReplaceFieldValue = false); + } // namespace mongo::timeseries |