diff options
7 files changed, 189 insertions, 83 deletions
diff --git a/jstests/concurrency/fsm_workloads/analyze_shard_key.js b/jstests/concurrency/fsm_workloads/analyze_shard_key.js index 32fc90bdf98..5a7db07bf82 100644 --- a/jstests/concurrency/fsm_workloads/analyze_shard_key.js +++ b/jstests/concurrency/fsm_workloads/analyze_shard_key.js @@ -757,7 +757,11 @@ var $config = extendWorkload($config, function($config, $super) { $config.states.disableQuerySampling = function disableQuerySampling(db, collName) { print("Starting disableQuerySampling state"); const ns = db.getName() + "." + collName; - assert.commandWorked(db.adminCommand({configureQueryAnalyzer: ns, mode: "off"})); + // If query sampling is off, this command is expected to fail with an IllegalOperation + // error. + assert.commandWorkedOrFailedWithCode( + db.adminCommand({configureQueryAnalyzer: ns, mode: "off"}), + ErrorCodes.IllegalOperation); print("Finished disableQuerySampling state"); }; diff --git a/jstests/sharding/analyze_shard_key/configure_query_analyzer_persistence.js b/jstests/sharding/analyze_shard_key/configure_query_analyzer_persistence.js index 81d49288310..9bffcd1657a 100644 --- a/jstests/sharding/analyze_shard_key/configure_query_analyzer_persistence.js +++ b/jstests/sharding/analyze_shard_key/configure_query_analyzer_persistence.js @@ -10,75 +10,122 @@ "use strict"; load("jstests/libs/uuid_util.js"); // for 'extractUUIDFromObject' - -/** - * Returns test cases for all combinations of options for the configureQueryAnalyzer command. - */ -function makeTestCases() { - const testCases = []; - for (const mode of ["off", "full"]) { - for (const sampleRate of [null, -1.0, 0.0, 0.2]) { - let testCase = Object.assign({}, {command: {mode, sampleRate}}); - if (sampleRate == null) { - delete testCase.command.sampleRate; - } - if ((mode == "off" && sampleRate !== null) || - (mode == "full" && - (sampleRate == null || typeof sampleRate !== "number" || sampleRate <= 0.0))) { - continue; // These cases are tested in configure_query_analyzer_basic.js. - } - testCases.push(testCase); - } +load("jstests/sharding/analyze_shard_key/libs/query_sampling_util.js"); + +function assertConfigQueryAnalyzerResponse(res, newConfig, oldConfig) { + assert.eq(res.newConfiguration, newConfig, res); + if (oldConfig) { + assert.eq(res.oldConfiguration, oldConfig); + } else { + assert(!res.hasOwnProperty("oldConfiguration"), oldConfig); } - return testCases; -} - -function assertConfigQueryAnalyzerResponse(res, mode, sampleRate) { - assert.eq(res.mode, mode); - assert.eq(res.sampleRate, sampleRate); } -function assertQueryAnalyzerConfigDoc(conn, dbName, collName, mode, sampleRate) { - const listCollRes = assert.commandWorked( - conn.getDB(dbName).runCommand({listCollections: 1, filter: {name: collName}})); - const uuid = listCollRes.cursor.firstBatch[0].info.uuid; - const doc = conn.getCollection("config.queryAnalyzers").findOne({_id: uuid}); +function assertQueryAnalyzerConfigDoc(conn, ns, collUuid, mode, sampleRate, startTime, stopTime) { + const doc = conn.getCollection("config.queryAnalyzers").findOne({_id: collUuid}); + assert.eq(doc.ns, ns, doc); assert.eq(doc.mode, mode, doc); - if (mode == "off") { - assert.eq(doc.hasOwnProperty("sampleRate"), false, doc); - } else if (mode == "full") { - assert.eq(doc.sampleRate, sampleRate, doc); + assert.eq(doc.sampleRate, sampleRate, doc); + assert(doc.hasOwnProperty("startTime"), doc); + if (startTime) { + assert.eq(doc.startTime, startTime, doc); } + assert.eq(doc.hasOwnProperty("stopTime"), mode == "off", doc); + if (stopTime) { + assert.eq(doc.stopTime, stopTime, doc); + } + return doc; } -function assertNoQueryAnalyzerConfigDoc(conn, dbName, collName) { - const ns = dbName + "." + collName; - const doc = conn.getCollection("config.queryAnalyzers").findOne({ns: ns}); +function assertNoQueryAnalyzerConfigDoc(conn, collUuid) { + const doc = conn.getCollection("config.queryAnalyzers").findOne({_id: collUuid}); assert.eq(doc, null, doc); } -function testPersistingConfiguration(conn, testCases) { +function testPersistingConfiguration(conn) { const dbName = "testDb-" + extractUUIDFromObject(UUID()); const collName = "testColl"; const ns = dbName + "." + collName; + const db = conn.getDB(dbName); - assert.commandWorked(conn.getDB(dbName).runCommand({insert: collName, documents: [{x: 1}]})); - - testCases.forEach(testCase => { - jsTest.log( - `Testing that the configureQueryAnalyzer command persists the configuration correctly ${ - tojson(testCase)}`); - - const res = conn.adminCommand({ - configureQueryAnalyzer: ns, - mode: testCase.command.mode, - sampleRate: testCase.command.sampleRate - }); - assert.commandWorked(res); - assertConfigQueryAnalyzerResponse(res, testCase.command.mode, testCase.command.sampleRate); - assertQueryAnalyzerConfigDoc( - conn, dbName, collName, testCase.command.mode, testCase.command.sampleRate); - }); + assert.commandWorked(db.createCollection(collName)); + const collUuid = QuerySamplingUtil.getCollectionUuid(db, collName); + + jsTest.log( + `Testing that the configureQueryAnalyzer command persists the configuration correctly ${ + tojson({dbName, collName, collUuid})}`); + + // Run a configureQueryAnalyzer command to disable query sampling. Verify that the command + // fails since query sampling is not even active. + const mode0 = "off"; + assert.commandFailedWithCode(conn.adminCommand({configureQueryAnalyzer: ns, mode: mode0}), + ErrorCodes.IllegalOperation); + assertNoQueryAnalyzerConfigDoc(conn, collUuid); + + // Run a configureQueryAnalyzer command to enable query sampling. + const mode1 = "full"; + const sampleRate1 = 100; + const res1 = assert.commandWorked( + conn.adminCommand({configureQueryAnalyzer: ns, mode: mode1, sampleRate: sampleRate1})); + const doc1 = assertQueryAnalyzerConfigDoc(conn, ns, collUuid, mode1, sampleRate1); + assertConfigQueryAnalyzerResponse(res1, {mode: mode1, sampleRate: sampleRate1} /* newConfig */); + + // Run a configureQueryAnalyzer command to modify the sample rate. Verify that the 'startTime' + // remains the same. + const mode2 = "full"; + const sampleRate2 = 0.2; + const res2 = assert.commandWorked( + conn.adminCommand({configureQueryAnalyzer: ns, mode: mode2, sampleRate: sampleRate2})); + assertQueryAnalyzerConfigDoc(conn, ns, collUuid, mode2, sampleRate2, doc1.startTime); + assertConfigQueryAnalyzerResponse(res2, + {mode: mode2, sampleRate: sampleRate2} /* newConfig */, + {mode: mode1, sampleRate: sampleRate1} /* oldConfig */); + + // Run a configureQueryAnalyzer command to disable query sampling. + const mode3 = "off"; + const res3 = assert.commandWorked(conn.adminCommand({configureQueryAnalyzer: ns, mode: mode3})); + assertQueryAnalyzerConfigDoc(conn, ns, collUuid, mode3, sampleRate2, doc1.startTime); + assertConfigQueryAnalyzerResponse(res3, + {mode: mode3} /* newConfig */, + {mode: mode2, sampleRate: sampleRate2} /* oldConfig */); + + // Run a configureQueryAnalyzer command to re-enable query sampling. Verify that the 'startTime' + // is new. + const mode4 = "full"; + const sampleRate4 = 1; + const res4 = assert.commandWorked( + conn.adminCommand({configureQueryAnalyzer: ns, mode: mode4, sampleRate: sampleRate4})); + const doc4 = assertQueryAnalyzerConfigDoc(conn, ns, collUuid, mode4, sampleRate4); + assert.gt(doc4.startTime, doc1.startTime, doc4); + assertConfigQueryAnalyzerResponse(res4, + {mode: mode4, sampleRate: sampleRate4} /* newConfig */, + {mode: mode3, sampleRate: sampleRate2} /* oldConfig */); + + // Retry the previous configureQueryAnalyzer command. Verify that the 'startTime' remains the + // same. + const res4Retry = assert.commandWorked( + conn.adminCommand({configureQueryAnalyzer: ns, mode: mode4, sampleRate: sampleRate4})); + assertQueryAnalyzerConfigDoc(conn, ns, collUuid, mode4, sampleRate4, doc4.startTime); + assertConfigQueryAnalyzerResponse(res4Retry, + {mode: mode4, sampleRate: sampleRate4} /* newConfig */, + {mode: mode4, sampleRate: sampleRate4} /* oldConfig */); + + // Run a configureQueryAnalyzer command to disable query sampling. Verify that the 'sampleRate' + // doesn't get unset. + const mode5 = "off"; + const res5 = assert.commandWorked(conn.adminCommand({configureQueryAnalyzer: ns, mode: mode5})); + const doc5 = + assertQueryAnalyzerConfigDoc(conn, ns, collUuid, mode5, sampleRate4, doc4.startTime); + assertConfigQueryAnalyzerResponse(res5, + {mode: mode5} /* newConfig */, + {mode: mode4, sampleRate: sampleRate4} /* oldConfig */); + + // Retry the previous configureQueryAnalyzer command. Verify that the 'stopTime' remains the + // same. + assert.commandFailedWithCode(conn.adminCommand({configureQueryAnalyzer: ns, mode: mode5}), + ErrorCodes.IllegalOperation); + assertQueryAnalyzerConfigDoc( + conn, ns, collUuid, mode5, sampleRate4, doc4.startTime, doc5.stopTime); } function testDeletingConfigurations(conn, {dropDatabase, dropCollection, isShardedColl, st}) { @@ -103,13 +150,14 @@ function testDeletingConfigurations(conn, {dropDatabase, dropCollection, isShard assert.commandWorked( st.s0.adminCommand({moveChunk: ns, find: {x: 0}, to: st.shard1.shardName})); } + const collUuid = QuerySamplingUtil.getCollectionUuid(db, collName); const mode = "full"; const sampleRate = 0.5; const res = assert.commandWorked(conn.adminCommand({configureQueryAnalyzer: ns, mode, sampleRate})); - assertConfigQueryAnalyzerResponse(res, mode, sampleRate); - assertQueryAnalyzerConfigDoc(conn, dbName, collName, mode, sampleRate); + assertConfigQueryAnalyzerResponse(res, {mode, sampleRate}); + assertQueryAnalyzerConfigDoc(conn, ns, collUuid, mode, sampleRate); if (dropDatabase) { assert.commandWorked(db.dropDatabase()); @@ -117,15 +165,13 @@ function testDeletingConfigurations(conn, {dropDatabase, dropCollection, isShard assert(coll.drop()); } - assertNoQueryAnalyzerConfigDoc(conn, dbName, collName); + assertNoQueryAnalyzerConfigDoc(conn, collUuid); } -const testCases = makeTestCases(); - { const st = new ShardingTest({shards: 2, rs: {nodes: 1}}); - testPersistingConfiguration(st.s, testCases); + testPersistingConfiguration(st.s); // TODO (SERVER-70479): Make sure that dropDatabase and dropCollection delete the // config.queryAnalyzers doc for the collection being dropped. // testDeletingConfigurations(st.s, {dropDatabase: true, isShardedColl: false, st}); @@ -142,7 +188,7 @@ const testCases = makeTestCases(); rst.initiate(); const primary = rst.getPrimary(); - testPersistingConfiguration(primary, testCases); + testPersistingConfiguration(primary); // TODO (SERVER-70479): Make sure that dropDatabase and dropCollection delete the // config.queryAnalyzers doc for the collection being dropped. // testDeletingConfigurations(primary, {dropDatabase: true, isShardedColl: false}); diff --git a/jstests/sharding/analyze_shard_key/refresh_sample_rates.js b/jstests/sharding/analyze_shard_key/refresh_sample_rates.js index 1b41b1e1aa9..da73e1a2dfc 100644 --- a/jstests/sharding/analyze_shard_key/refresh_sample_rates.js +++ b/jstests/sharding/analyze_shard_key/refresh_sample_rates.js @@ -132,9 +132,6 @@ function testBasic(createConnFn, rst, samplerNames) { numQueriesExecutedPerSecond: 0 })); assert.eq(0, res2.configurations.length); - - assert.commandWorked(conn.adminCommand({configureQueryAnalyzer: ns0, mode: "off"})); - assert.commandWorked(conn.adminCommand({configureQueryAnalyzer: ns1, mode: "off"})); } function testFailover(createConnFn, rst, samplerNames) { diff --git a/src/mongo/db/s/configure_query_analyzer_cmd.cpp b/src/mongo/db/s/configure_query_analyzer_cmd.cpp index d8a0fef1e55..b212ab96f49 100644 --- a/src/mongo/db/s/configure_query_analyzer_cmd.cpp +++ b/src/mongo/db/s/configure_query_analyzer_cmd.cpp @@ -173,29 +173,73 @@ public: } auto collUuid = uassertStatusOK(validateCollectionOptions(opCtx, nss)); - QueryAnalyzerDocument qad; - qad.setNs(nss); - qad.setCollectionUuid(collUuid); - qad.setConfiguration(newConfig); - // TODO SERVER-69804: Implement start/stop timestamp in config.queryAnalyzers - // document. LOGV2(6915001, "Persisting query analyzer configuration", logAttrs(nss), "collectionUUID"_attr = collUuid, "mode"_attr = mode, "sampleRate"_attr = sampleRate); - PersistentTaskStore<QueryAnalyzerDocument> store{ - NamespaceString::kConfigQueryAnalyzersNamespace}; - store.upsert( - opCtx, - BSON(QueryAnalyzerDocument::kCollectionUuidFieldName << qad.getCollectionUuid()), - qad.toBSON(), - WriteConcerns::kMajorityWriteConcernNoTimeout); + + write_ops::FindAndModifyCommandRequest request( + NamespaceString::kConfigQueryAnalyzersNamespace); + + using doc = QueryAnalyzerDocument; + + auto currentTime = opCtx->getServiceContext()->getFastClockSource()->now(); + if (mode == QueryAnalyzerModeEnum::kOff) { + request.setUpsert(false); + // If the mode is 'off', do not perform the update since that would overwrite the + // existing stop time. + request.setQuery(BSON( + doc::kCollectionUuidFieldName + << collUuid << doc::kModeFieldName + << BSON("$ne" << QueryAnalyzerMode_serializer(QueryAnalyzerModeEnum::kOff)))); + + std::vector<BSONObj> updates; + updates.push_back( + BSON("$set" << BSON(doc::kModeFieldName + << QueryAnalyzerMode_serializer(QueryAnalyzerModeEnum::kOff) + << doc::kStopTimeFieldName << currentTime))); + request.setUpdate(write_ops::UpdateModification(updates)); + } else { + request.setUpsert(true); + request.setQuery(BSON(doc::kCollectionUuidFieldName << collUuid)); + + std::vector<BSONObj> updates; + BSONObjBuilder setBuilder; + setBuilder.appendElements(BSON(doc::kCollectionUuidFieldName + << collUuid << doc::kNsFieldName << nss.toString())); + setBuilder.appendElements(newConfig.toBSON()); + // If the mode remains the same, keep the original start time. Otherwise, set a new + // start time. + setBuilder.append( + doc::kStartTimeFieldName, + BSON("$cond" << BSON("if" << BSON("$ne" << BSON_ARRAY( + ("$" + doc::kModeFieldName) + << QueryAnalyzerMode_serializer(mode))) + << "then" << currentTime << "else" + << ("$" + doc::kStartTimeFieldName)))); + updates.push_back(BSON("$set" << setBuilder.obj())); + updates.push_back(BSON("$unset" << doc::kStopTimeFieldName)); + request.setUpdate(write_ops::UpdateModification(updates)); + } + request.setWriteConcern(WriteConcerns::kMajorityWriteConcernNoTimeout.toBSON()); + + DBDirectClient client(opCtx); + auto writeResult = client.findAndModify(request); Response response; - // TODO SERVER-70019: Make configQueryAnalyzer return old configuration. response.setNewConfiguration(newConfig); + if (auto preImageDoc = writeResult.getValue()) { + auto oldConfig = QueryAnalyzerConfiguration::parse( + IDLParserContext("configureQueryAnalyzer"), *preImageDoc); + response.setOldConfiguration(oldConfig); + } else { + uassert(ErrorCodes::IllegalOperation, + "Attempted to disable query sampling but query sampling was not active", + mode != QueryAnalyzerModeEnum::kOff); + } + return response; } diff --git a/src/mongo/db/s/query_analysis_coordinator_test.cpp b/src/mongo/db/s/query_analysis_coordinator_test.cpp index 76bd5a20c34..b99e060250a 100644 --- a/src/mongo/db/s/query_analysis_coordinator_test.cpp +++ b/src/mongo/db/s/query_analysis_coordinator_test.cpp @@ -80,6 +80,10 @@ protected: configuration.setMode(mode); configuration.setSampleRate(sampleRate); doc.setConfiguration(configuration); + doc.setStartTime(now()); + if (mode == QueryAnalyzerModeEnum::kOff) { + doc.setStopTime(now()); + } return doc; } @@ -90,6 +94,7 @@ protected: ASSERT(it != configurations.end()); auto& configuration = it->second; ASSERT_EQ(configuration.getNs(), analyzerDoc.getNs()); + ASSERT_EQ(configuration.getCollectionUuid(), analyzerDoc.getCollectionUuid()); ASSERT_EQ(configuration.getSampleRate(), *analyzerDoc.getSampleRate()); } diff --git a/src/mongo/s/analyze_shard_key_documents.idl b/src/mongo/s/analyze_shard_key_documents.idl index 503f3e89437..634c7a9b081 100644 --- a/src/mongo/s/analyze_shard_key_documents.idl +++ b/src/mongo/s/analyze_shard_key_documents.idl @@ -47,6 +47,13 @@ structs: ns: type: namespacestring description: "The namespace of the collection." + startTime: + type: date + description: "The time at which query sampling began." + stopTime: + type: date + description: "The time at which query sampling ended." + optional: true inline_chained_structs: true chained_structs: QueryAnalyzerConfiguration: configuration diff --git a/src/mongo/s/configure_query_analyzer_cmd.idl b/src/mongo/s/configure_query_analyzer_cmd.idl index 667b964afa8..addd37eb0fa 100644 --- a/src/mongo/s/configure_query_analyzer_cmd.idl +++ b/src/mongo/s/configure_query_analyzer_cmd.idl @@ -37,9 +37,12 @@ structs: configureQueryAnalyzerResponse: description: "The response for the 'configureQueryAnalyzer' command." strict: false - inline_chained_structs: true - chained_structs: - QueryAnalyzerConfiguration: newConfiguration + fields: + newConfiguration: + type: QueryAnalyzerConfiguration + oldConfiguration: + type: QueryAnalyzerConfiguration + optional: true commands: configureQueryAnalyzer: |