diff options
4 files changed, 166 insertions, 2 deletions
diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml index eb5a187f792..384459fe07e 100644 --- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml +++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml @@ -107,6 +107,8 @@ selector: # Enable if SERVER-36966 is backported to v3.6 - jstests/sharding/mr_output_sharded_validation.js - jstests/sharding/shard_collection_existing_zones.js + # Enable when BACKPORT-4652 is released to v3.6 + - jstests/sharding/server_status_crud_metrics.js executor: config: diff --git a/jstests/sharding/server_status_crud_metrics.js b/jstests/sharding/server_status_crud_metrics.js new file mode 100644 index 00000000000..5e6619a3219 --- /dev/null +++ b/jstests/sharding/server_status_crud_metrics.js @@ -0,0 +1,127 @@ +/** + * Tests for the 'metrics.query' section of the mongoS serverStatus response dealing with CRUD + * operations. + */ + +(function() { + "use strict"; + + const st = new ShardingTest({shards: 2}); + 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.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})); + + // 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}), + ErrorCodes.ImmutableField); + assert.commandFailedWithCode( + testDB.coll.update({_id: 1}, {$set: {x: 2, $invalidField: 4}}, {multi: false}), + ErrorCodes.DollarPrefixedFieldName); + + let mongosServerStatus = testDB.adminCommand({serverStatus: 1}); + + // Verify that the above four updates incremented the metric counter. + 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})); + + // 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})); + + // 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})); + + // Shouldn't increment the metric when routing fails. + assert.commandFailedWithCode(testDB.coll.update({}, {$set: {x: 2}}, {multi: false}), + ErrorCodes.ShardKeyNotFound); + assert.commandFailedWithCode(testDB.coll.update({_id: 1}, {$set: {x: 2}}, {upsert: true}), + ErrorCodes.ShardKeyNotFound); + + // Shouldn't increment the metrics for unsharded collection. + assert.commandWorked(unshardedColl.update({_id: "missing"}, {$set: {a: 1}}, {multi: false})); + assert.commandWorked(unshardedColl.update({_id: 1}, {$set: {a: 2}}, {multi: false})); + + // Shouldn't incement the metrics when query had invalid operator. + assert.commandFailedWithCode( + testDB.coll.update({_id: 1, $invalidOperator: 1}, {$set: {a: 2}}, {multi: false}), + ErrorCodes.BadValue); + + mongosServerStatus = testDB.adminCommand({serverStatus: 1}); + + // Verify that only the first four upserts incremented the metric counter. + assert.eq(4, mongosServerStatus.metrics.query.updateOneOpStyleBroadcastWithExactIDCount); + + // Verification for 'upsertReplacementCannotTargetByQueryCount' metric. + + // Should increment the metric when the upsert can target single shard based on shard key in + // replacement doc but query doesn't have shard key. + assert.commandWorked(testDB.coll.update({_id: "missing"}, {x: 1, a: 2}, {upsert: true})); + assert.commandWorked(testDB.coll.update({}, {x: 1, a: 1}, {upsert: true})); + + // Should increment the metric, even though the update subsequently fails on the shard when it + // attempts an invalid modification of the _id. + assert.commandFailedWithCode(testDB.coll.update({_id: 1}, {x: 1, a: 1, _id: 2}, {upsert: true}), + ErrorCodes.ImmutableField); + + mongosServerStatus = testDB.adminCommand({serverStatus: 1}); + + // Verify that the above three updates incremented the metric counter. + assert.eq(3, mongosServerStatus.metrics.query.upsertReplacementCannotTargetByQueryCount); + + // Shouldn't increment the metrics when query has shard key. + assert.commandWorked(testDB.coll.update({x: 1}, {x: 1, a: 1}, {upsert: true})); + assert.commandWorked(testDB.coll.update({x: 1, _id: 1}, {x: 1, a: 1}, {upsert: true})); + + // Shouldn't increment the metrics for opstyle upserts. + assert.commandWorked(testDB.coll.update({x: 1, _id: 1}, {$set: {x: 1, a: 1}}, {upsert: true})); + assert.commandFailedWithCode(testDB.coll.update({_id: 1}, {$set: {x: 1, a: 1}}, {upsert: true}), + ErrorCodes.ShardKeyNotFound); + + // Shouldn't increment the metric when the query is invalid. + assert.commandFailedWithCode( + testDB.coll.update({_id: 1, $invalidOperator: 5}, {x: 1, a: 1, _id: 2}, {upsert: true}), + ErrorCodes.BadValue); + + // Shouldn't increment the metric when routing fails. + assert.commandFailedWithCode(testDB.coll.update({x: 1}, {a: 2}, {upsert: true}), + ErrorCodes.ShardKeyNotFound); + assert.commandFailedWithCode(testDB.coll.update({x: 1, _id: 1}, {a: 2}, {upsert: true}), + ErrorCodes.ShardKeyNotFound); + + // Shouldn't increment the metrics for unsharded collection. + assert.commandWorked(unshardedColl.update({_id: "missing"}, {x: 1, a: 2}, {upsert: true})); + assert.commandWorked(unshardedColl.update({}, {x: 1, a: 1}, {upsert: true})); + assert.commandFailedWithCode( + unshardedColl.update({_id: 1}, {x: 1, a: 1, _id: 2}, {upsert: true}), + ErrorCodes.ImmutableField); + + mongosServerStatus = testDB.adminCommand({serverStatus: 1}); + + // Verify that only the first three upserts incremented the metric counter. + assert.eq(3, mongosServerStatus.metrics.query.upsertReplacementCannotTargetByQueryCount); + + st.stop(); +})(); diff --git a/src/mongo/s/write_ops/SConscript b/src/mongo/s/write_ops/SConscript index 593d65a4d23..c339571b8ac 100644 --- a/src/mongo/s/write_ops/SConscript +++ b/src/mongo/s/write_ops/SConscript @@ -32,6 +32,7 @@ env.Library( 'write_op.cpp', ], LIBDEPS=[ + '$BUILD_DIR/mongo/db/commands/server_status_core', '$BUILD_DIR/mongo/s/async_requests_sender', '$BUILD_DIR/mongo/s/commands/cluster_commands_helpers', 'batch_write_types', diff --git a/src/mongo/s/write_ops/chunk_manager_targeter.cpp b/src/mongo/s/write_ops/chunk_manager_targeter.cpp index 72c63da21a3..6cd063bf1f7 100644 --- a/src/mongo/s/write_ops/chunk_manager_targeter.cpp +++ b/src/mongo/s/write_ops/chunk_manager_targeter.cpp @@ -34,6 +34,8 @@ #include "mongo/s/write_ops/chunk_manager_targeter.h" +#include "mongo/base/counter.h" +#include "mongo/db/commands/server_status_metric.h" #include "mongo/db/matcher/extensions_callback_noop.h" #include "mongo/db/query/canonical_query.h" #include "mongo/db/query/collation/collation_index_key.h" @@ -53,6 +55,18 @@ enum CompareResult { CompareResult_Unknown, CompareResult_GTE, CompareResult_LT const ShardKeyPattern virtualIdShardKey(BSON("_id" << 1)); +// Tracks the number of {multi:false} updates with an exact match on _id that are broadcasted to +// multiple shards. +Counter64 updateOneOpStyleBroadcastWithExactIDCount; +ServerStatusMetricField<Counter64> updateOneOpStyleBroadcastWithExactIDStats( + "query.updateOneOpStyleBroadcastWithExactIDCount", &updateOneOpStyleBroadcastWithExactIDCount); + +// Tracks the number of replacement-style upserts which do not have an exact shard key match in the +// query. +Counter64 upsertReplacementCannotTargetByQueryCount; +ServerStatusMetricField<Counter64> upsertReplacementCannotTargetByQueryStats( + "query.upsertReplacementCannotTargetByQueryCount", &upsertReplacementCannotTargetByQueryCount); + /** * There are two styles of update expressions: * @@ -384,8 +398,23 @@ StatusWith<std::vector<ShardEndpoint>> ChunkManagerTargeter::targetUpdate( // Target the shard key, query, or replacement doc if (!shardKey.isEmpty()) { try { - return std::vector<ShardEndpoint>{ - _targetShardKey(shardKey, collation, (query.objsize() + updateExpr.objsize()))}; + auto targetShard = + _targetShardKey(shardKey, collation, (query.objsize() + updateExpr.objsize())); + + // If the request is a replacement-style upsert and we were able to target based on + // replacement doc, we want to track the requests for which shard key is not present in + // the query. + if (updateDoc.getUpsert() && updateType == UpdateType_Replacement) { + auto swShardKey = + _routingInfo->cm()->getShardKeyPattern().extractShardKeyFromQuery(opCtx, query); + + // If the query is valid and the shard key extracted is empty, record the event in + // our serverStatus metrics. + if (swShardKey.isOK() && swShardKey.getValue().isEmpty()) { + upsertReplacementCannotTargetByQueryCount.increment(1); + } + } + return std::vector<ShardEndpoint>{std::move(targetShard)}; } catch (const DBException&) { // This update is potentially not constrained to a single shard } @@ -443,6 +472,11 @@ StatusWith<std::vector<ShardEndpoint>> ChunkManagerTargeter::targetUpdate( } if (updateType == UpdateType_OpStyle) { + // If the request is {multi:false}, then this is an update which we are broadcasting to + // multiple shards by exact _id. Record this event in our serverStatus metrics. + if (_routingInfo->cm() && !updateDoc.getMulti()) { + updateOneOpStyleBroadcastWithExactIDCount.increment(1); + } return _targetQuery(opCtx, query, collation); } else { return _targetDoc(opCtx, updateExpr, collation); |