From 553067485afc74087de94ef11298a02298524fae Mon Sep 17 00:00:00 2001 From: Adi Zaimi Date: Thu, 10 Nov 2022 20:52:04 +0000 Subject: SERVER-67795: Add serverstatus counters for updateMany, deleteMany calls(6.1) --- jstests/sharding/server_status_crud_metrics.js | 29 +++--- jstests/sharding/update_delete_many_metrics.js | 122 +++++++++++++++++++++++++ src/mongo/db/commands/write_commands.cpp | 10 ++ src/mongo/db/stats/counters.cpp | 3 + src/mongo/db/stats/counters.h | 5 + src/mongo/s/chunk_manager_targeter.cpp | 9 ++ 6 files changed, 163 insertions(+), 15 deletions(-) create mode 100644 jstests/sharding/update_delete_many_metrics.js diff --git a/jstests/sharding/server_status_crud_metrics.js b/jstests/sharding/server_status_crud_metrics.js index 4f83d7e4b6e..99821f78b4c 100644 --- a/jstests/sharding/server_status_crud_metrics.js +++ b/jstests/sharding/server_status_crud_metrics.js @@ -26,15 +26,14 @@ assert.commandWorked(unshardedColl.insert({x: 1, _id: 1})); // Verification for 'updateOneOpStyleBroadcastWithExactIDCount' metric. // Should increment the metric as the update cannot target single shard and are {multi:false}. -assert.commandWorked(testDB.coll.update({_id: "missing"}, {$set: {a: 1}}, {multi: false})); -assert.commandWorked(testDB.coll.update({_id: 1}, {$set: {a: 2}}, {multi: false})); +assert.commandWorked(testColl.update({_id: "missing"}, {$set: {a: 1}}, {multi: false})); +assert.commandWorked(testColl.update({_id: 1}, {$set: {a: 2}}, {multi: false})); // Should increment the metric because we broadcast by _id, even though the update subsequently // fails on the individual shard. -assert.commandFailedWithCode(testDB.coll.update({_id: 1}, {$set: {x: 2}}, {multi: false}), 31025); -assert.commandFailedWithCode( - testDB.coll.update({_id: 1}, {$set: {x: 12}, $hello: 1}, {multi: false}), - ErrorCodes.FailedToParse); +assert.commandFailedWithCode(testColl.update({_id: 1}, {$set: {x: 2}}, {multi: false}), 31025); +assert.commandFailedWithCode(testColl.update({_id: 1}, {$set: {x: 12}, $hello: 1}, {multi: false}), + ErrorCodes.FailedToParse); let mongosServerStatus = testDB.adminCommand({serverStatus: 1}); @@ -42,21 +41,21 @@ let mongosServerStatus = testDB.adminCommand({serverStatus: 1}); assert.eq(4, mongosServerStatus.metrics.query.updateOneOpStyleBroadcastWithExactIDCount); // Shouldn't increment the metric when {multi:true}. -assert.commandWorked(testDB.coll.update({_id: 1}, {$set: {a: 3}}, {multi: true})); -assert.commandWorked(testDB.coll.update({}, {$set: {a: 3}}, {multi: true})); +assert.commandWorked(testColl.update({_id: 1}, {$set: {a: 3}}, {multi: true})); +assert.commandWorked(testColl.update({}, {$set: {a: 3}}, {multi: true})); // Shouldn't increment the metric when update can target single shard. -assert.commandWorked(testDB.coll.update({x: 11}, {$set: {a: 2}}, {multi: false})); -assert.commandWorked(testDB.coll.update({x: 1}, {$set: {a: 2}}, {multi: false})); +assert.commandWorked(testColl.update({x: 11}, {$set: {a: 2}}, {multi: false})); +assert.commandWorked(testColl.update({x: 1}, {$set: {a: 2}}, {multi: false})); // Shouldn't increment the metric for replacement style updates. -assert.commandWorked(testDB.coll.update({_id: 1}, {x: 1, a: 2})); -assert.commandWorked(testDB.coll.update({x: 1}, {x: 1, a: 1})); +assert.commandWorked(testColl.update({_id: 1}, {x: 1, a: 2})); +assert.commandWorked(testColl.update({x: 1}, {x: 1, a: 1})); // Shouldn't increment the metric when routing fails. -assert.commandFailedWithCode(testDB.coll.update({}, {$set: {x: 2}}, {multi: false}), +assert.commandFailedWithCode(testColl.update({}, {$set: {x: 2}}, {multi: false}), ErrorCodes.InvalidOptions); -assert.commandFailedWithCode(testDB.coll.update({_id: 1}, {$set: {x: 2}}, {upsert: true}), +assert.commandFailedWithCode(testColl.update({_id: 1}, {$set: {x: 2}}, {upsert: true}), ErrorCodes.ShardKeyNotFound); // Shouldn't increment the metrics for unsharded collection. @@ -65,7 +64,7 @@ assert.commandWorked(unshardedColl.update({_id: 1}, {$set: {a: 2}}, {multi: fals // Shouldn't incement the metrics when query had invalid operator. assert.commandFailedWithCode( - testDB.coll.update({_id: 1, $invalidOperator: 1}, {$set: {a: 2}}, {multi: false}), + testColl.update({_id: 1, $invalidOperator: 1}, {$set: {a: 2}}, {multi: false}), ErrorCodes.BadValue); mongosServerStatus = testDB.adminCommand({serverStatus: 1}); diff --git a/jstests/sharding/update_delete_many_metrics.js b/jstests/sharding/update_delete_many_metrics.js new file mode 100644 index 00000000000..af7daf59d20 --- /dev/null +++ b/jstests/sharding/update_delete_many_metrics.js @@ -0,0 +1,122 @@ +/** + * Tests for the 'metrics.query' section of the mongos and mongod serverStatus response verifying + * counters for updateMany and deleteMany + * @tags: [multiversion_incompatible] + */ + +(function() { +"use strict"; + +{ + const st = new ShardingTest({shards: 2, rs: {nodes: 2}}); + const mongodConns = []; + st.rs0.nodes.forEach(node => mongodConns.push(node)); + st.rs1.nodes.forEach(node => mongodConns.push(node)); + + const testDB = st.s.getDB("test"); + const testColl = testDB.coll; + const unshardedColl = testDB.unsharded; + + assert.commandWorked(st.s0.adminCommand({enableSharding: testDB.getName()})); + st.ensurePrimaryShard(testDB.getName(), st.shard0.shardName); + + // Shard testColl on {x:1}, split it at {x:0}, and move chunk {x:1} to shard1. + st.shardColl(testColl, {x: 1}, {x: 0}, {x: 1}); + + // Insert one document on each shard. + assert.commandWorked(testColl.insert({x: 1, _id: 1})); + assert.commandWorked(testColl.insert({x: -1, _id: 0})); + + assert.eq(2, testColl.countDocuments({})); + assert.commandWorked(unshardedColl.insert({x: 1, _id: 1})); + assert.eq(1, unshardedColl.countDocuments({})); + + let mongosServerStatus = testDB.adminCommand({serverStatus: 1}); + + // Verification for 'updateManyCount' metric. + assert.eq(0, mongosServerStatus.metrics.query.updateManyCount); + // Verification for 'deleteManyCount' metric. + assert.eq(0, mongosServerStatus.metrics.query.deleteManyCount); + + assert.commandWorked(unshardedColl.update({_id: 1}, {$set: {a: 2}}, {multi: false})); + assert.commandWorked(testColl.update({_id: 1}, {$set: {a: 2}}, {multi: false})); + // 3 update with multi:true calls. + assert.commandWorked(unshardedColl.update({_id: 1}, {$set: {a: 2}}, {multi: true})); + assert.commandWorked(testColl.update({}, {$set: {a: 3}}, {multi: true})); + assert.commandWorked(testColl.update({_id: 1}, {$set: {a: 2}}, {multi: true})); + + // 2 updateMany calls. + assert.commandWorked(testColl.updateMany({}, {$set: {array: 'string', doc: 'string'}})); + assert.commandWorked(unshardedColl.updateMany({}, {$set: {array: 'string', doc: 'string'}})); + + // Use deleteMany to delete one of the documents. + const result = testColl.deleteMany({_id: 1}); + assert.commandWorked(result); + assert.eq(1, result.deletedCount); + assert.eq(1, testColl.countDocuments({})); + // Next call will not increase count. + assert.commandWorked(testColl.deleteOne({_id: 1})); + + // Use deleteMany to delete one document in the unsharded collection. + assert.commandWorked(unshardedColl.deleteMany({_id: 1})); + // Next call will not increase count. + assert.commandWorked(unshardedColl.deleteOne({_id: 1})); + + mongosServerStatus = testDB.adminCommand({serverStatus: 1}); + + // Verification for 'updateManyCount' metric. + assert.eq(5, mongosServerStatus.metrics.query.updateManyCount); + // Verification for 'deleteManyCount' metric. + assert.eq(2, mongosServerStatus.metrics.query.deleteManyCount); + + st.stop(); +} + +{ + const rst = new ReplSetTest({nodes: 2}); + rst.startSet(); + rst.initiate(); + + const primary = rst.getPrimary(); + const testDB = primary.getDB("test"); + const testColl = testDB.coll; + const unshardedColl = testDB.unsharded; + + // Insert one document on each shard. + assert.commandWorked(testColl.insert({x: 1, _id: 1})); + assert.commandWorked(testColl.insert({x: -1, _id: 0})); + assert.eq(2, testColl.countDocuments({})); + + let mongosServerStatus = testDB.adminCommand({serverStatus: 1}); + + // Verification for 'updateManyCount' metric. + assert.eq(0, mongosServerStatus.metrics.query.updateManyCount); + // Verification for 'deleteManyCount' metric. + assert.eq(0, mongosServerStatus.metrics.query.deleteManyCount); + + assert.commandWorked(testColl.update({_id: 1}, {$set: {a: 2}}, {multi: false})); + // 3 update with multi:true calls. + assert.commandWorked(testColl.update({}, {$set: {a: 3}}, {multi: true})); + assert.commandWorked(testColl.update({_id: 1}, {$set: {a: 2}}, {multi: true})); + + // 2 updateMany call. + assert.commandWorked(testColl.updateMany({}, {$set: {array: 'string', doc: 'string'}})); + + // Use deleteMany to delete one of the documents. + const result = testColl.deleteMany({_id: 1}); + assert.commandWorked(result); + assert.eq(1, result.deletedCount); + assert.eq(1, testColl.countDocuments({})); + // Next call will not increase count. + assert.commandWorked(testColl.deleteOne({_id: 1})); + + mongosServerStatus = testDB.adminCommand({serverStatus: 1}); + + // Verification for 'updateManyCount' metric. + assert.eq(3, mongosServerStatus.metrics.query.updateManyCount); + // Verification for 'deleteManyCount' metric. + assert.eq(1, mongosServerStatus.metrics.query.deleteManyCount); + + rst.stopSet(); +} +})(); diff --git a/src/mongo/db/commands/write_commands.cpp b/src/mongo/db/commands/write_commands.cpp index 0ddf1c96316..daf86320f19 100644 --- a/src/mongo/db/commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands.cpp @@ -1559,6 +1559,9 @@ public: if (update.getArrayFilters()) { CmdUpdate::updateMetrics.incrementExecutedWithArrayFilters(); } + if (update.getMulti()) { + updateManyCount.increment(1); + } } return updateReply; @@ -1717,6 +1720,13 @@ public: std::move(reply), &deleteReply); + // Collect metrics. + for (auto&& deletes : request().getDeletes()) { + if (deletes.getMulti()) { + deleteManyCount.increment(1); + } + } + return deleteReply; } catch (const DBException& ex) { NotPrimaryErrorTracker::get(opCtx->getClient()).recordError(ex.code()); diff --git a/src/mongo/db/stats/counters.cpp b/src/mongo/db/stats/counters.cpp index 44328202169..aa603de6216 100644 --- a/src/mongo/db/stats/counters.cpp +++ b/src/mongo/db/stats/counters.cpp @@ -326,4 +326,7 @@ OperatorCounters operatorCountersMatchExpressions{"operatorCounters.match."}; OperatorCounters operatorCountersGroupAccumulatorExpressions{"operatorCounters.groupAccumulators."}; OperatorCounters operatorCountersWindowAccumulatorExpressions{ "operatorCounters.windowAccumulators."}; +CounterMetric updateManyCount("query.updateManyCount"); +CounterMetric deleteManyCount("query.deleteManyCount"); + } // namespace mongo diff --git a/src/mongo/db/stats/counters.h b/src/mongo/db/stats/counters.h index 9e8cbbbe46d..6a3d5dc4b4b 100644 --- a/src/mongo/db/stats/counters.h +++ b/src/mongo/db/stats/counters.h @@ -424,4 +424,9 @@ extern OperatorCounters operatorCountersGroupAccumulatorExpressions; // Global counters for accumulator expressions apply to $setWindowFields. extern OperatorCounters operatorCountersWindowAccumulatorExpressions; +// Track the number of {multi:true} updates. +extern CounterMetric updateManyCount; +// Track the number of deleteMany calls. +extern CounterMetric deleteManyCount; + } // namespace mongo diff --git a/src/mongo/s/chunk_manager_targeter.cpp b/src/mongo/s/chunk_manager_targeter.cpp index 5d964f74e51..5584b754717 100644 --- a/src/mongo/s/chunk_manager_targeter.cpp +++ b/src/mongo/s/chunk_manager_targeter.cpp @@ -39,6 +39,7 @@ #include "mongo/db/query/canonical_query.h" #include "mongo/db/query/collation/collation_index_key.h" #include "mongo/db/query/collation/collator_factory_interface.h" +#include "mongo/db/stats/counters.h" #include "mongo/db/timeseries/timeseries_constants.h" #include "mongo/db/timeseries/timeseries_options.h" #include "mongo/db/timeseries/timeseries_update_delete_util.h" @@ -384,6 +385,10 @@ std::vector ChunkManagerTargeter::targetUpdate(OperationContext* const auto& updateOp = itemRef.getUpdate(); + if (updateOp.getMulti()) { + updateManyCount.increment(1); + } + // If the collection is not sharded, forward the update to the primary shard. if (!_cm.isSharded()) { // TODO (SERVER-51070): Remove the boost::none when the config server can support @@ -500,6 +505,10 @@ std::vector ChunkManagerTargeter::targetDelete(OperationContext* itemRef.getLet(), itemRef.getLegacyRuntimeConstants()); + if (deleteOp.getMulti()) { + deleteManyCount.increment(1); + } + BSONObj deleteQuery = deleteOp.getQ(); BSONObj shardKey; if (_cm.isSharded()) { -- cgit v1.2.1