From 778c374279c2cf8f04878d0d93cc19fd2d67bf7b Mon Sep 17 00:00:00 2001 From: Justin Seyster Date: Thu, 8 Dec 2022 22:34:09 +0000 Subject: [PATCH 1/5] PERF-3060 [WiP] Add post-condition verification for CRUDActor metrics --- src/cast_core/src/CrudActor.cpp | 106 ++++++++++----- src/metrics/include/metrics/PostCondition.hpp | 127 ++++++++++++++++++ src/metrics/include/metrics/operation.hpp | 27 +++- src/workloads/execution/CheckMetricsTest.yml | 71 ++++++++++ 4 files changed, 294 insertions(+), 37 deletions(-) create mode 100644 src/metrics/include/metrics/PostCondition.hpp create mode 100644 src/workloads/execution/CheckMetricsTest.yml diff --git a/src/cast_core/src/CrudActor.cpp b/src/cast_core/src/CrudActor.cpp index 9422a29bdf..0f77e20857 100644 --- a/src/cast_core/src/CrudActor.cpp +++ b/src/cast_core/src/CrudActor.cpp @@ -69,7 +69,6 @@ ThrowMode decodeThrowMode(const Node& operation, PhaseContext& phaseContext) { bsoncxx::document::value emptyDoc = bsoncxx::from_json("{}"); - // A large number of subclasses have // - metrics::Operation // - mongocxx::collection @@ -79,11 +78,15 @@ bsoncxx::document::value emptyDoc = bsoncxx::from_json("{}"); // duplication. struct BaseOperation { ThrowMode throwMode; + PostCondition checker; using MaybeDoc = std::optional; - explicit BaseOperation(PhaseContext& phaseContext, const Node& operation) - : throwMode{decodeThrowMode(operation, phaseContext)} {} + explicit BaseOperation(PhaseContext& phaseContext, + const Node& operation, + const Node& postCondition) + : throwMode{decodeThrowMode(operation, phaseContext)}, + checker{postCondition.maybe().value_or(PostCondition{})} {} template void doBlock(metrics::Operation& op, F&& f) { @@ -101,7 +104,7 @@ struct BaseOperation { return; } } - ctx.success(); + ctx.conditionalSuccess(checker); } virtual void run(mongocxx::client_session& session) = 0; @@ -109,6 +112,7 @@ struct BaseOperation { }; using OpCallback = std::function(const Node&, + const Node&, bool, mongocxx::collection, metrics::Operation, @@ -118,13 +122,18 @@ using OpCallback = std::function(const Node&, std::unordered_map getOpConstructors(); struct WriteOperation : public BaseOperation { - WriteOperation(PhaseContext& phaseContext, const Node& operation) - : BaseOperation(phaseContext, operation) {} + WriteOperation(PhaseContext& phaseContext, const Node& operation, const Node& postCondition) + : BaseOperation(phaseContext, operation, postCondition) {} virtual mongocxx::model::write getModel() = 0; }; -using WriteOpCallback = std::function( - const Node&, bool, mongocxx::collection, metrics::Operation, PhaseContext&, ActorId)>; +using WriteOpCallback = std::function(const Node&, + const Node&, + bool, + mongocxx::collection, + metrics::Operation, + PhaseContext&, + ActorId)>; // Not technically "crud" but it was easy to add and made // a few of the tests easier to write (by allowing inserts @@ -139,12 +148,13 @@ struct CreateIndexOperation : public BaseOperation { bool _onSession; CreateIndexOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _collection(std::move(collection)), _operation{operation}, _onSession{onSession}, @@ -168,12 +178,13 @@ struct CreateIndexOperation : public BaseOperation { struct InsertOneOperation : public WriteOperation { InsertOneOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : WriteOperation(context, opNode), + : WriteOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -210,12 +221,13 @@ struct InsertOneOperation : public WriteOperation { struct UpdateOneOperation : public WriteOperation { UpdateOneOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : WriteOperation(context, opNode), + : WriteOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -256,12 +268,13 @@ struct UpdateOneOperation : public WriteOperation { struct UpdateManyOperation : public WriteOperation { UpdateManyOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : WriteOperation(context, opNode), + : WriteOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -302,12 +315,13 @@ struct UpdateManyOperation : public WriteOperation { struct DeleteOneOperation : public WriteOperation { DeleteOneOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : WriteOperation(context, opNode), + : WriteOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -342,12 +356,13 @@ struct DeleteOneOperation : public WriteOperation { struct DeleteManyOperation : public WriteOperation { DeleteManyOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : WriteOperation(context, opNode), + : WriteOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -380,12 +395,13 @@ struct DeleteManyOperation : public WriteOperation { struct ReplaceOneOperation : public WriteOperation { ReplaceOneOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : WriteOperation(context, opNode), + : WriteOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -428,12 +444,14 @@ struct ReplaceOneOperation : public WriteOperation { template C baseCallback = [](const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) -> std::unique_ptr

{ - return std::make_unique(opNode, onSession, collection, operation, context, id); + return std::make_unique( + opNode, postCondition, onSession, collection, operation, context, id); }; // Maps the WriteCommand name to the constructor of the designated Operation struct. @@ -466,12 +484,13 @@ std::unordered_map bulkWriteConstructors = { struct BulkWriteOperation : public BaseOperation { BulkWriteOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation} { @@ -497,9 +516,10 @@ struct BulkWriteOperation : public BaseOperation { } auto createWriteOp = writeOpConstructor->second; auto& yamlCommand = writeOp["OperationCommand"]; + auto& postCondition = writeOp["PostCondition"]; bool onSession = yamlCommand["OnSession"].maybe().value_or(_onSession); _writeOps.push_back( - createWriteOp(writeOp, onSession, _collection, _operation, context, id)); + createWriteOp(writeOp, postCondition, onSession, _collection, _operation, context, id)); } void run(mongocxx::client_session& session) override { @@ -546,12 +566,13 @@ struct BulkWriteOperation : public BaseOperation { */ struct CountDocumentsOperation : public BaseOperation { CountDocumentsOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -584,12 +605,13 @@ struct CountDocumentsOperation : public BaseOperation { struct EstimatedDocumentCountOperation : public BaseOperation { EstimatedDocumentCountOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation} { @@ -626,12 +648,13 @@ struct EstimatedDocumentCountOperation : public BaseOperation { struct FindOperation : public BaseOperation { FindOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -665,12 +688,13 @@ struct FindOperation : public BaseOperation { struct FindOneOperation : public BaseOperation { FindOneOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -703,12 +727,13 @@ struct FindOneOperation : public BaseOperation { struct AggregateOperation : public BaseOperation { AggregateOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -719,6 +744,7 @@ struct AggregateOperation : public BaseOperation { } void run(mongocxx::client_session& session) override { + BOOST_LOG_TRIVIAL(info) << "Running aggregate operation"; auto pipeline = pipeline_helpers::makePipeline(_pipelineGenerator); this->doBlock(_operation, [&](metrics::OperationContext& ctx) { auto cursor = _onSession ? _collection.aggregate(session, pipeline, _options) @@ -741,12 +767,13 @@ struct AggregateOperation : public BaseOperation { struct FindOneAndUpdateOperation : public BaseOperation { FindOneAndUpdateOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -779,12 +806,13 @@ struct FindOneAndUpdateOperation : public BaseOperation { struct FindOneAndDeleteOperation : public BaseOperation { FindOneAndDeleteOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -814,12 +842,13 @@ struct FindOneAndDeleteOperation : public BaseOperation { struct FindOneAndReplaceOperation : public BaseOperation { FindOneAndReplaceOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation}, @@ -863,12 +892,13 @@ struct FindOneAndReplaceOperation : public BaseOperation { struct InsertManyOperation : public BaseOperation { InsertManyOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation} { @@ -932,12 +962,13 @@ struct InsertManyOperation : public BaseOperation { struct StartTransactionOperation : public BaseOperation { StartTransactionOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), _operation{operation} { + : BaseOperation(context, opNode, postCondition), _operation{operation} { if (!opNode.isMap()) return; if (opNode["Options"]) { @@ -966,12 +997,13 @@ struct StartTransactionOperation : public BaseOperation { struct CommitTransactionOperation : public BaseOperation { CommitTransactionOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), _operation{operation} {} + : BaseOperation(context, opNode, postCondition), _operation{operation} {} void run(mongocxx::client_session& session) override { this->doBlock(_operation, [&](metrics::OperationContext& ctx) { @@ -1002,12 +1034,13 @@ struct CommitTransactionOperation : public BaseOperation { struct WithTransactionOperation : public BaseOperation { WithTransactionOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation} { @@ -1054,13 +1087,15 @@ struct WithTransactionOperation : public BaseOperation { } auto createOp = opConstructor->second; auto& yamlCommand = txnOp["OperationCommand"]; + auto& yamlPostCondition = txnOp["PostCondition"]; // operations can override withTransaction's OnSession value // This behavior is hard to test. // The CrudActorYamlTests suite exercises both branches of this boolean // but doesn't assert the behavior. // Be careful when making changes around this code. bool onSession = yamlCommand["OnSession"].maybe().value_or(_onSession); - _txnOps.push_back(createOp(yamlCommand, onSession, _collection, _operation, context, id)); + _txnOps.push_back(createOp( + yamlCommand, yamlPostCondition, onSession, _collection, _operation, context, id)); } mongocxx::collection _collection; @@ -1081,12 +1116,13 @@ struct WithTransactionOperation : public BaseOperation { struct SetReadConcernOperation : public BaseOperation { SetReadConcernOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _collection{std::move(collection)}, _operation{operation} { _readConcern = opNode["ReadConcern"].to(); @@ -1117,12 +1153,13 @@ struct SetReadConcernOperation : public BaseOperation { */ struct DropOperation : public BaseOperation { DropOperation(const Node& opNode, + const Node& postCondition, bool onSession, mongocxx::collection collection, metrics::Operation operation, PhaseContext& context, ActorId id) - : BaseOperation(context, opNode), + : BaseOperation(context, opNode, postCondition), _onSession{onSession}, _collection{std::move(collection)}, _operation{operation} { @@ -1488,6 +1525,7 @@ struct CrudActor::PhaseConfig { ActorId id) const { auto collection = (*client)[dbName][name]; auto& yamlCommand = node["OperationCommand"]; + auto& yamlPostCondition = node["PostCondition"]; auto opName = node["OperationName"].to(); auto opMetricsName = node["OperationMetricsName"].maybe().value_or(opName); auto onSession = yamlCommand["OnSession"].maybe().value_or(false); @@ -1519,6 +1557,7 @@ struct CrudActor::PhaseConfig { stm << *metricsName << "." << opMetricsName; return opCreator(yamlCommand, + yamlPostCondition, onSession, collection, phaseContext.actor().operation(stm.str(), id), @@ -1527,6 +1566,7 @@ struct CrudActor::PhaseConfig { } return opCreator(yamlCommand, + yamlPostCondition, onSession, collection, perPhaseMetrics ? phaseContext.operation(opMetricsName, id) diff --git a/src/metrics/include/metrics/PostCondition.hpp b/src/metrics/include/metrics/PostCondition.hpp new file mode 100644 index 0000000000..1f616c9447 --- /dev/null +++ b/src/metrics/include/metrics/PostCondition.hpp @@ -0,0 +1,127 @@ +// Copyright 2022-present MongoDB Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef HEADER_7CD54FB0_E953_4F3C_B7F4_5BDA6B299878_INCLUDED +#define HEADER_7CD54FB0_E953_4F3C_B7F4_5BDA6B299878_INCLUDED + +#include + +namespace genny { + +class PostConditionException : public std::logic_error { +public: + using std::logic_error::logic_error; +}; + +enum class MetricRequirement { + kNumDocuments, + kBytes, +}; + +enum class Comparison { + kGT, +}; + +struct PostCondition { + // The default PostCondition is a tautology. + PostCondition() = default; + + PostCondition(const Node& node) { + if (node.isSequence()) { + for (const auto& [k, condition] : node) { + addCondition(condition); + } + } else { + addCondition(node); + } + } + + void check(long long ops, long long bytes) const { + for (const auto& req : requirements) { + long long observedValue = [&]() { + switch (req.metric) { + case MetricRequirement::kNumDocuments: + return ops; + case MetricRequirement::kBytes: + return bytes; + } + throw std::logic_error("impossible"); + }(); + + if (!req.predicate.evaluateFn(observedValue, req.requiredValue)) { + BOOST_LOG_TRIVIAL(error) + << "Expected metric " << req.predicate.symbol << " " << req.requiredValue + << " but actual value was " << observedValue << "."; + BOOST_THROW_EXCEPTION( + PostConditionException("Operation post-condition not granted.")); + } + } + } + +private: + struct Predicate { + std::string key; + std::string symbol; + std::function evaluateFn; + }; + + struct Requirement { + MetricRequirement metric; + const Predicate& predicate; + long long requiredValue; + + Requirement(MetricRequirement metric, const Predicate& predicate, long long requiredValue) + : metric(metric), predicate(predicate), requiredValue(requiredValue) {} + }; + + void addCondition(const Node& node) { + static const char* metricKey = "Metric"; + static const char* documentMetric = "documents"; + static const char* bytesMetric = "bytes"; + + static const std::vector predicates = { + {"EQ", "==", [](long long left, long long right) { return left == right; }}, + {"NE", "!=", [](long long left, long long right) { return left != right; }}, + {"LT", "==", [](long long left, long long right) { return left < right; }}, + {"LTE", "==", [](long long left, long long right) { return left <= right; }}, + {"GT", "==", [](long long left, long long right) { return left > right; }}, + {"GTE", "==", [](long long left, long long right) { return left >= right; }}, + }; + + auto metricName = node[metricKey].maybe(); + MetricRequirement metric; + if (!metricName) { + BOOST_THROW_EXCEPTION(InvalidConfigurationException( + "'PostCondition' expects a 'Metric' field of string type.")) + } else if (metricName == documentMetric) { + metric = MetricRequirement::kNumDocuments; + } else if (metricName == bytesMetric) { + metric = MetricRequirement::kBytes; + } else { + BOOST_THROW_EXCEPTION(InvalidConfigurationException("Unexpected metric")); + } + + for (const auto& predicate : predicates) { + if (auto requiredValue = node[predicate.key].maybe()) { + requirements.emplace_back(metric, predicate, *requiredValue); + } + } + } + + std::vector requirements; +}; + +} // namespace genny + +#endif // HEADER_7CD54FB0_E953_4F3C_B7F4_5BDA6B299878_INCLUDED diff --git a/src/metrics/include/metrics/operation.hpp b/src/metrics/include/metrics/operation.hpp index 050ac24b2b..f79bcf6464 100644 --- a/src/metrics/include/metrics/operation.hpp +++ b/src/metrics/include/metrics/operation.hpp @@ -29,6 +29,7 @@ #include #include +#include #include #include @@ -333,7 +334,7 @@ class OperationContextT final : private boost::noncopyable { * @warning After calling success() it is illegal to call any methods on this instance. */ void success() { - reportOutcome(OutcomeType::kSuccess); + reportOutcome(OutcomeType::kSuccess, ClockSource::now()); } /** @@ -343,7 +344,26 @@ class OperationContextT final : private boost::noncopyable { * @warning After calling failure() it is illegal to call any methods on this instance. */ void failure() { - reportOutcome(OutcomeType::kFailure); + reportOutcome(OutcomeType::kFailure, ClockSource::now()); + } + + /** + * Report the operation as having succeeded if 'postCondition' holds for the reported metrics. + * Otherwise, report the operations as having failed and then propagate the exception thrown by + * the post-condition check. + * + * @warning After calling conditionalSuccess() it is illegal to call any methods on this + * instance. + */ + void conditionalSuccess(const PostCondition& postCondition = {}) { + auto finished = ClockSource::now(); + try { + postCondition.check(_event.number, _event.size); + reportOutcome(OutcomeType::kSuccess, finished); + } catch (const PostConditionException&) { + reportOutcome(OutcomeType::kFailure, finished); + throw; + } } /** @@ -356,8 +376,7 @@ class OperationContextT final : private boost::noncopyable { } private: - void reportOutcome(OutcomeType outcome) { - auto finished = ClockSource::now(); + void reportOutcome(OutcomeType outcome, time_point finished) { _event.duration = finished - _started; _event.outcome = outcome; diff --git a/src/workloads/execution/CheckMetricsTest.yml b/src/workloads/execution/CheckMetricsTest.yml new file mode 100644 index 0000000000..57d744556c --- /dev/null +++ b/src/workloads/execution/CheckMetricsTest.yml @@ -0,0 +1,71 @@ +SchemaVersion: 2018-07-01 +Owner: "@mongodb/query-execution" +Description: | + PoC of metrics checking. + +Keywords: +- aggregate + +GlobalDefaults: + Database: &Database test + Collection: &Collection Collection0 + DocumentCount: &DocumentCount 100 + Threads: &Threads 1 + MaxPhases: &MaxPhases 3 + +Actors: +# Clear any pre-existing collection state. +- Name: ClearCollection + Type: CrudActor + Database: *Database + Threads: *Threads + Phases: + OnlyActiveInPhases: + Active: [0] + NopInPhasesUpTo: *MaxPhases + PhaseConfig: + Repeat: 1 + Collection: *Collection + Operations: + - OperationName: drop + +- Name: InsertData + Type: Loader + Threads: 1 + Phases: + OnlyActiveInPhases: + Active: [1] + NopInPhasesUpTo: *MaxPhases + PhaseConfig: + Repeat: 1 + Database: *Database + Threads: 1 + CollectionCount: 1 + DocumentCount: *DocumentCount + BatchSize: 1000 + Document: + int: &integer {^RandomInt: {min: 0, max: 10}} + int2: *integer + +- Name: CrudActorAggregateExample + Type: CrudActor + Database: *Database + Threads: 1 + Phases: + OnlyActiveInPhases: + Active: [2] + NopInPhasesUpTo: *MaxPhases + PhaseConfig: + Repeat: 1 + Collection: *Collection + Operations: + - OperationName: aggregate + OperationMetricsName: PipelineWithoutOptions + OperationCommand: + Pipeline: [{$group: {_id: "$int", sum: {$sum: "$int2"}}}] + PostCondition: + - Metric: documents + GT: 4 + LT: 50 + - Metric: bytes + EQ: 100 From fe54aa70a4eab6e0e95ff022a9a7b1191b49ddc4 Mon Sep 17 00:00:00 2001 From: Justin Seyster Date: Wed, 25 Jan 2023 00:27:00 +0000 Subject: [PATCH 2/5] Add unit tests for CrudActor PostCondition --- src/cast_core/test/CrudActorYamlTests.yml | 213 ++++++++++++++++++++++ 1 file changed, 213 insertions(+) diff --git a/src/cast_core/test/CrudActorYamlTests.yml b/src/cast_core/test/CrudActorYamlTests.yml index 4950f6d57e..78c043c7cf 100644 --- a/src/cast_core/test/CrudActorYamlTests.yml +++ b/src/cast_core/test/CrudActorYamlTests.yml @@ -716,3 +716,216 @@ Tests: Pipeline: {$group: {_id: "$a", sum: {$sum: "$b"}}} Error: | .*'Pipeline' must be an array.* + + - Description: Find command that fulfills EQ post-condition executes as expected. + Operations: + - OperationName: insertMany + OperationCommand: + Documents: + - { a: 1 } + - { a: 1 } + - { b: 1 } + - OperationName: find + OperationCommand: + Filter: { a : 1 } + PostCondition: + - Metric: documents + EQ: 2 + OutcomeCounts: + - Filter: {a: 1} + Count: 2 + - Filter: {b: 1} + Count: 1 + + - Description: Unfulfilled EQ post-condition results in error. + Operations: + - OperationName: insertMany + OperationCommand: + Documents: + - { a: 1 } + - { a: 1 } + - { b: 1 } + - OperationName: find + OperationCommand: + Filter: { a : 1 } + PostCondition: + - Metric: documents + EQ: 5 + Error: | + .*post-condition not granted.* + + - Description: Find command that fulfills range post-condition executes as expected. + Operations: + - OperationName: insertMany + OperationCommand: + Documents: + - { a: 1 } + - { a: 1 } + - { b: 1 } + - OperationName: find + OperationCommand: + Filter: { a : 1 } + PostCondition: + - Metric: bytes + GT: 50 + LT: 100 + OutcomeCounts: + - Filter: {a: 1} + Count: 2 + - Filter: {b: 1} + Count: 1 + + - Description: Unfulfilled range post-condition results in error. + Operations: + - OperationName: insertMany + OperationCommand: + Documents: + - { a: 1 } + - { a: 1 } + - { b: 1 } + - OperationName: find + OperationCommand: + Filter: { b : 1 } + PostCondition: + - Metric: bytes + GT: 50 + LT: 100 + Error: | + .*post-condition not granted.* + + - Description: Find command that fulfills multiple post-conditions executes as expected. + Operations: + - OperationName: insertMany + OperationCommand: + Documents: + - { a: 1 } + - { a: 1 } + - { b: 1 } + - OperationName: find + OperationCommand: + Filter: { a : 1 } + PostCondition: + - Metric: documents + EQ: 2 + - Metric: bytes + GT: 50 + LT: 100 + OutcomeCounts: + - Filter: {a: 1} + Count: 2 + - Filter: {b: 1} + Count: 1 + + - Description: One unfulfilled post-condition out of multiple expected post-conditions results in error. + Operations: + - OperationName: insertMany + OperationCommand: + Documents: + - { a: 1 } + - { a: 1 } + - { b: 1 } + - OperationName: find + OperationCommand: + Filter: { a : 1 } + PostCondition: + - Metric: documents + EQ: 2 + - Metric: bytes + GT: 500 + Error: | + .*post-condition not granted.* + + - Description: Multiple insert commands fulfilling their post-conditions all execute as expected. + Operations: + - OperationName: insertMany + OperationCommand: + Documents: + - { a: 1 } + - { a: 1 } + - { b: 1 } + PostCondition: + - Metric: documents + EQ: 3 + - OperationName: insertMany + OperationCommand: + Documents: + - { b: 1 } + - { b: 1 } + - { c: 1 } + PostCondition: + - Metric: documents + EQ: 3 + OutcomeCounts: + - Filter: {a: 1} + Count: 2 + - Filter: {b: 1} + Count: 3 + - Filter: {c: 1} + Count: 1 + + - Description: BulkWrite post-condition applies to sum of constituent write operations. + Operations: + - OperationName: bulkWrite + OperationCommand: + WriteOperations: + - WriteCommand: insertOne + Document: { a: 1 } + - WriteCommand: deleteOne + Filter: { a: 1 } + - WriteCommand: insertOne + Document: { a: 2 } + PostCondition: + - Metric: documents + EQ: 3 + OutcomeData: + - {a: 2} + + - Description: Operations with transactions execute as expected, withTransaction metric values are 0. + Operations: + - OperationName: withTransaction + OperationCommand: + OperationsInTransaction: + - OperationName: insertOne + OperationCommand: + Document: {a: 100} + - OperationName: findOneAndReplace + OperationCommand: + Filter: {a: 100} + Replacement: {a: 30} + PostCondition: + - Metric: documents + EQ: 1 + PostCondition: + - Metric: documents + EQ: 0 + - Metric: bytes + EQ: 0 + OutcomeCounts: + - Filter: {a: 100} + Count: 0 + - Filter: {a: 30} + Count: 1 + AssertNumTransactionsCommitted: 0 + + - Description: Unfulfilled post-condition within transaction results in error. + Operations: + - OperationName: withTransaction + OperationCommand: + OperationsInTransaction: + - OperationName: insertOne + OperationCommand: + Document: {a: 100} + - OperationName: findOneAndReplace + OperationCommand: + Filter: {a: 100} + Replacement: {a: 30} + PostCondition: + - Metric: documents + GT: 100 + PostCondition: + - Metric: documents + EQ: 0 + - Metric: bytes + EQ: 0 + Error: | + .*post-condition not granted.* From 8de8df46af76720d60132788e5e8215d5e10c3da Mon Sep 17 00:00:00 2001 From: Justin Seyster Date: Wed, 25 Jan 2023 00:28:59 +0000 Subject: [PATCH 3/5] Add example of using PostCondition for after-the-fact validation of write volume --- src/workloads/execution/CheckMetricsTest.yml | 61 +++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/src/workloads/execution/CheckMetricsTest.yml b/src/workloads/execution/CheckMetricsTest.yml index 57d744556c..ba4bc3bc56 100644 --- a/src/workloads/execution/CheckMetricsTest.yml +++ b/src/workloads/execution/CheckMetricsTest.yml @@ -11,7 +11,7 @@ GlobalDefaults: Collection: &Collection Collection0 DocumentCount: &DocumentCount 100 Threads: &Threads 1 - MaxPhases: &MaxPhases 3 + MaxPhases: &MaxPhases 6 Actors: # Clear any pre-existing collection state. @@ -67,5 +67,62 @@ Actors: - Metric: documents GT: 4 LT: 50 + +- Name: CrudWriterExample + Type: CrudActor + Database: *Database + Threads: 1 + Phases: + OnlyActiveInPhases: + Active: [3] + NopInPhasesUpTo: *MaxPhases + PhaseConfig: + Repeat: 10 + Collection: *Collection + Operations: + - OperationName: insertMany + OperationMetricsName: InsertLargeString + OperationCommand: + Documents: + - {str: {^FastRandomString: {length: 1024}}} + +# Make sure that the "LargeStringInserts" workload wrote the expected amount of data. +- Name: ValidateLargeStringInserts + Type: CrudActor + Database: *Database + Threads: 1 + Phases: + OnlyActiveInPhases: + Active: [4] + NopInPhasesUpTo: *MaxPhases + PhaseConfig: + Repeat: 1 + Collection: *Collection + Operations: + - OperationName: find + OperationMetricsName: LargeStringBytes + OperationCommand: + Filter: {str: {$ne: null}} + PostCondition: - Metric: bytes - EQ: 100 + GT: 10000 + +- Name: AATest + Type: AssertiveActor + Database: *Database + Threads: 1 + Phases: + OnlyActiveInPhases: + Active: [5] + NopInPhasesUpTo: *MaxPhases + PhaseConfig: + Repeat: 1 + Message: Test message + Actual: + aggregate: *Collection + pipeline: [{$project: {size: {$bsonSize: "$$ROOT"}}}, {$group: {_id: null, totalSize: {$sum: "$size"}}}, {$project: {passed: {$gt: ["$totalSize", 15000]}}}] + cursor: {} + Expected: + aggregate: 1 + pipeline: [{$documents: [{passed: true}]}] + cursor: {} From 909105a91e9e96906585e0c1725ebd54f7673097 Mon Sep 17 00:00:00 2001 From: Justin Seyster Date: Wed, 25 Jan 2023 23:47:21 +0000 Subject: [PATCH 4/5] Comments to document the PostCondition class --- src/metrics/include/metrics/PostCondition.hpp | 79 +++++++++++++++---- 1 file changed, 64 insertions(+), 15 deletions(-) diff --git a/src/metrics/include/metrics/PostCondition.hpp b/src/metrics/include/metrics/PostCondition.hpp index 1f616c9447..dc00efdcde 100644 --- a/src/metrics/include/metrics/PostCondition.hpp +++ b/src/metrics/include/metrics/PostCondition.hpp @@ -29,10 +29,35 @@ enum class MetricRequirement { kBytes, }; -enum class Comparison { - kGT, -}; - +/** + * A PostCondition added to a CrudActor Operation checks the metrics of the operation immediately + * after it runs, so that it can be marked as failing if it does not meet expectations for the + * intended test scenario. For example, a post-condition ensuring that a find command is returning + * the expected number of documents can quickly identify a spurious performance improvement caused + * by a bug in query evaluation. A post-condition can also check the state of a collection, ensuring + * that it is large enough to appropriately stress target code paths, when attached to a query that + * scans the entire collection. + * + * Example: + * ```yaml + * Actors: + * - Name: InsertOne + * Type: CrudActor + * Database: test + * Phases: + * - Collection: test + * Operations: + * - OperationName: insertOne + * OperationCommand: + * Document: {a: "value"} + * PostCondition: + * - Metric: documents + * EQ: 1 + * - Metric: bytes + * LT: 20 + * GT: 5 + * ``` + */ struct PostCondition { // The default PostCondition is a tautology. PostCondition() = default; @@ -47,6 +72,10 @@ struct PostCondition { } } + /** + * Checks if the 'ops' and 'bytes' metrics for the execution of a CRUD operation meet the + * requirements and throws an exception if they do not. + */ void check(long long ops, long long bytes) const { for (const auto& req : requirements) { long long observedValue = [&]() { @@ -70,33 +99,48 @@ struct PostCondition { } private: - struct Predicate { + /** + * A binary comparison relation on 'long long' ints (e.g., ==, <) + */ + struct Relation { + // The YAML key used to include this relation in a PostCondition specification. std::string key; + + // The symbol to use when outputting user messages about a comparison result. std::string symbol; + + // Implementation that returns true if the relation holds (meeting the post-condition + // requirement) for a pair of values. std::function evaluateFn; }; struct Requirement { MetricRequirement metric; - const Predicate& predicate; + const Relation& relation; long long requiredValue; Requirement(MetricRequirement metric, const Predicate& predicate, long long requiredValue) - : metric(metric), predicate(predicate), requiredValue(requiredValue) {} + : metric(metric), relation(relation), requiredValue(requiredValue) {} }; + /** + * Parse one block from the list of blocks in a YAML PostCondition into one or more entries in + * the 'requirements' list. + */ void addCondition(const Node& node) { static const char* metricKey = "Metric"; static const char* documentMetric = "documents"; static const char* bytesMetric = "bytes"; - static const std::vector predicates = { + // The parser uses this table to translate the YAML key (e.g., "EQ") for a comparison into a + // function that can perform the comparison. + static const std::vector relations = { {"EQ", "==", [](long long left, long long right) { return left == right; }}, {"NE", "!=", [](long long left, long long right) { return left != right; }}, - {"LT", "==", [](long long left, long long right) { return left < right; }}, - {"LTE", "==", [](long long left, long long right) { return left <= right; }}, - {"GT", "==", [](long long left, long long right) { return left > right; }}, - {"GTE", "==", [](long long left, long long right) { return left >= right; }}, + {"LT", "<", [](long long left, long long right) { return left < right; }}, + {"LTE", "<=", [](long long left, long long right) { return left <= right; }}, + {"GT", ">", [](long long left, long long right) { return left > right; }}, + {"GTE", ">=", [](long long left, long long right) { return left >= right; }}, }; auto metricName = node[metricKey].maybe(); @@ -112,13 +156,18 @@ struct PostCondition { BOOST_THROW_EXCEPTION(InvalidConfigurationException("Unexpected metric")); } - for (const auto& predicate : predicates) { - if (auto requiredValue = node[predicate.key].maybe()) { - requirements.emplace_back(metric, predicate, *requiredValue); + for (const auto& relation : relations) { + if (auto requiredValue = node[relation.key].maybe()) { + requirements.emplace_back(metric, relation, *requiredValue); } } } + /* + * A list of requirements that must all be met for the post-condition to be fulfilled. Each + * requirement specifies the metric to check, a reference value to compare the metric to, and an + * arithmetic relation (e.g, ==, <) to compare with. + */ std::vector requirements; }; From d6461148637e612ed6238f52e3fbbcc09ad10924 Mon Sep 17 00:00:00 2001 From: Justin Seyster Date: Thu, 26 Jan 2023 00:41:08 +0000 Subject: [PATCH 5/5] Changes that accidentally got left out of previous commits --- src/metrics/include/metrics/PostCondition.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/metrics/include/metrics/PostCondition.hpp b/src/metrics/include/metrics/PostCondition.hpp index dc00efdcde..687b6f792d 100644 --- a/src/metrics/include/metrics/PostCondition.hpp +++ b/src/metrics/include/metrics/PostCondition.hpp @@ -88,9 +88,9 @@ struct PostCondition { throw std::logic_error("impossible"); }(); - if (!req.predicate.evaluateFn(observedValue, req.requiredValue)) { + if (!req.relation.evaluateFn(observedValue, req.requiredValue)) { BOOST_LOG_TRIVIAL(error) - << "Expected metric " << req.predicate.symbol << " " << req.requiredValue + << "Expected metric " << req.relation.symbol << " " << req.requiredValue << " but actual value was " << observedValue << "."; BOOST_THROW_EXCEPTION( PostConditionException("Operation post-condition not granted.")); @@ -119,7 +119,7 @@ struct PostCondition { const Relation& relation; long long requiredValue; - Requirement(MetricRequirement metric, const Predicate& predicate, long long requiredValue) + Requirement(MetricRequirement metric, const Relation& relation, long long requiredValue) : metric(metric), relation(relation), requiredValue(requiredValue) {} }; @@ -147,7 +147,7 @@ struct PostCondition { MetricRequirement metric; if (!metricName) { BOOST_THROW_EXCEPTION(InvalidConfigurationException( - "'PostCondition' expects a 'Metric' field of string type.")) + "'PostCondition' expects a 'Metric' field of string type.")); } else if (metricName == documentMetric) { metric = MetricRequirement::kNumDocuments; } else if (metricName == bytesMetric) {