diff options
author | Nick Zolnierz <nicholas.zolnierz@mongodb.com> | 2018-03-20 17:56:02 -0400 |
---|---|---|
committer | Nick Zolnierz <nicholas.zolnierz@mongodb.com> | 2018-03-20 17:57:31 -0400 |
commit | 2682dbfb28324406f6eded1f22f6e342a392ff13 (patch) | |
tree | 1a12a708dd45c159ce3a803090a7397e40fc68b7 /src | |
parent | 152e55c697613b0d99c619e9569fd6e57c303d2f (diff) | |
download | mongo-2682dbfb28324406f6eded1f22f6e342a392ff13.tar.gz |
Revert "SERVER-30005: remove $isolated/$atomic option"
This reverts commit cd950b113ee0d00e88036b2fe6306866c7ba27f9.
Diffstat (limited to 'src')
26 files changed, 264 insertions, 37 deletions
diff --git a/src/mongo/db/catalog/index_catalog_entry_impl.cpp b/src/mongo/db/catalog/index_catalog_entry_impl.cpp index 87d68175364..46abd0e3170 100644 --- a/src/mongo/db/catalog/index_catalog_entry_impl.cpp +++ b/src/mongo/db/catalog/index_catalog_entry_impl.cpp @@ -132,12 +132,14 @@ IndexCatalogEntryImpl::IndexCatalogEntryImpl(IndexCatalogEntry* const this_, new ExpressionContext(opCtx, _collator.get())); // Parsing the partial filter expression is not expected to fail here since the - // expression would have been successfully parsed upstream during index creation. + // expression would have been successfully parsed upstream during index creation. However, + // filters that were allowed in partial filter expressions prior to 3.6 may be present in + // the index catalog and must also successfully parse. StatusWithMatchExpression statusWithMatcher = MatchExpressionParser::parse(filter, std::move(expCtx), ExtensionsCallbackNoop(), - MatchExpressionParser::kBanAllSpecialFeatures); + MatchExpressionParser::AllowedFeatures::kIsolated); invariantOK(statusWithMatcher.getStatus()); _filterExpression = std::move(statusWithMatcher.getValue()); LOG(2) << "have filter expression for " << _ns << " " << _descriptor->indexName() << " " diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp index 525ace9c0e5..bb19de5ab76 100644 --- a/src/mongo/db/catalog/index_catalog_impl.cpp +++ b/src/mongo/db/catalog/index_catalog_impl.cpp @@ -686,12 +686,15 @@ Status IndexCatalogImpl::_isSpecOk(OperationContext* opCtx, const BSONObj& spec) new ExpressionContext(opCtx, collator.get())); // Parsing the partial filter expression is not expected to fail here since the - // expression would have been successfully parsed upstream during index creation. + // expression would have been successfully parsed upstream during index creation. However, + // filters that were allowed in partial filter expressions prior to 3.6 may be present in + // the index catalog and must also successfully parse (e.g., partial index filters with the + // $isolated/$atomic option). StatusWithMatchExpression statusWithMatcher = MatchExpressionParser::parse(filterElement.Obj(), std::move(expCtx), ExtensionsCallbackNoop(), - MatchExpressionParser::kBanAllSpecialFeatures); + MatchExpressionParser::kIsolated); if (!statusWithMatcher.isOK()) { return statusWithMatcher.getStatus(); } diff --git a/src/mongo/db/catalog/index_spec_validate_test.cpp b/src/mongo/db/catalog/index_spec_validate_test.cpp index 7f6f9beae0b..44f4e48b32c 100644 --- a/src/mongo/db/catalog/index_spec_validate_test.cpp +++ b/src/mongo/db/catalog/index_spec_validate_test.cpp @@ -806,7 +806,7 @@ TEST(IndexSpecPartialFilterTest, FailsIfPartialFilterContainsBannedFeature) { BSON("key" << BSON("field" << 1) << "name" << "indexName" << "partialFilterExpression" - << BSON("$jsonSchema" << BSONObj())), + << BSON("$isolated" << 1)), kTestNamespace, serverGlobalParams.featureCompatibility); ASSERT_EQ(result.getStatus(), ErrorCodes::QueryFeatureNotAllowed); diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp index 90806185039..6de36e438e8 100644 --- a/src/mongo/db/commands/find_cmd.cpp +++ b/src/mongo/db/commands/find_cmd.cpp @@ -154,7 +154,8 @@ public: std::move(qrStatus.getValue()), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures); + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); } @@ -271,7 +272,8 @@ public: std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures); + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); if (!statusWithCQ.isOK()) { return CommandHelpers::appendCommandStatus(result, statusWithCQ.getStatus()); } diff --git a/src/mongo/db/commands/geo_near_cmd.cpp b/src/mongo/db/commands/geo_near_cmd.cpp index 8aea0a035b4..d605fbb86b8 100644 --- a/src/mongo/db/commands/geo_near_cmd.cpp +++ b/src/mongo/db/commands/geo_near_cmd.cpp @@ -220,7 +220,8 @@ public: std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures); + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); if (!statusWithCQ.isOK()) { errmsg = "Can't parse filter / create query"; return false; diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index 0cf5d3314b4..d323bdd707c 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -1133,7 +1133,8 @@ void State::finalReduce(OperationContext* opCtx, CurOp* curOp, ProgressMeterHold std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures); + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); verify(statusWithCQ.isOK()); std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); @@ -1494,12 +1495,13 @@ public: const ExtensionsCallbackReal extensionsCallback(opCtx, &config.nss); const boost::intrusive_ptr<ExpressionContext> expCtx; - auto statusWithCQ = - CanonicalQuery::canonicalize(opCtx, - std::move(qr), - expCtx, - extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures); + auto statusWithCQ = CanonicalQuery::canonicalize( + opCtx, + std::move(qr), + expCtx, + extensionsCallback, + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); if (!statusWithCQ.isOK()) { uasserted(17238, "Can't canonicalize query " + config.filter.toString()); return 0; diff --git a/src/mongo/db/commands/plan_cache_commands_test.cpp b/src/mongo/db/commands/plan_cache_commands_test.cpp index 43d0f7d3830..fb0ed6c3e03 100644 --- a/src/mongo/db/commands/plan_cache_commands_test.cpp +++ b/src/mongo/db/commands/plan_cache_commands_test.cpp @@ -273,6 +273,24 @@ TEST(PlanCacheCommandsTest, Canonicalize) { ASSERT_NOT_EQUALS(planCache.computeKey(*query), planCache.computeKey(*projectionQuery)); } +TEST(PlanCacheCommandsTest, PlanCacheIgnoresIsolated) { + PlanCache planCache; + QueryTestServiceContext serviceContext; + auto opCtx = serviceContext.makeOperationContext(); + + // Query with $isolated should generate the same key as a query without $siolated. + auto statusWithCQ = + PlanCacheCommand::canonicalize(opCtx.get(), nss.ns(), fromjson("{query: {a: 1, b: 1}}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> query = std::move(statusWithCQ.getValue()); + + statusWithCQ = PlanCacheCommand::canonicalize( + opCtx.get(), nss.ns(), fromjson("{query: {a: 1, b: 1}, $isolated: 1}")); + ASSERT_OK(statusWithCQ.getStatus()); + unique_ptr<CanonicalQuery> queryWithIsolated = std::move(statusWithCQ.getValue()); + ASSERT_EQUALS(planCache.computeKey(*query), planCache.computeKey(*queryWithIsolated)); +} + /** * Tests for planCacheClear (single query shape) */ diff --git a/src/mongo/db/dbhelpers.cpp b/src/mongo/db/dbhelpers.cpp index cecdbccf6fb..d8645f93f36 100644 --- a/src/mongo/db/dbhelpers.cpp +++ b/src/mongo/db/dbhelpers.cpp @@ -120,7 +120,8 @@ RecordId Helpers::findOne(OperationContext* opCtx, std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures); + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); massertStatusOK(statusWithCQ.getStatus()); unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index 461f82fc736..74676d9293f 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -349,7 +349,9 @@ StatusWithMatchExpression parse(const BSONObj& obj, // A nullptr for 'parsedExpression' indicates that the particular operator should not // be added to 'root', because it is handled outside of the MatchExpressionParser // library. The following operators currently follow this convention: + // - $atomic is explicitly handled in CanonicalQuery::init() // - $comment has no action associated with the operator. + // - $isolated is explicitly handled in CanoncialQuery::init() if (parsedExpression.getValue().get()) { root->add(parsedExpression.getValue().release()); } @@ -399,6 +401,24 @@ StatusWithMatchExpression parse(const BSONObj& obj, return {std::move(root)}; } +StatusWithMatchExpression parseAtomicOrIsolated( + StringData name, + BSONElement elem, + const boost::intrusive_ptr<ExpressionContext>& expCtx, + const ExtensionsCallback* extensionsCallback, + MatchExpressionParser::AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel) { + if ((allowedFeatures & MatchExpressionParser::AllowedFeatures::kIsolated) == 0u) { + return {Status(ErrorCodes::QueryFeatureNotAllowed, + "$isolated ($atomic) is not allowed in this context")}; + } + if (currentLevel != DocumentParseLevel::kPredicateTopLevel) { + return { + Status(ErrorCodes::FailedToParse, "$isolated ($atomic) has to be at the top level")}; + } + return {nullptr}; +} + StatusWithMatchExpression parseComment(StringData name, BSONElement elem, const boost::intrusive_ptr<ExpressionContext>& expCtx, @@ -1817,10 +1837,12 @@ MONGO_INITIALIZER(PathlessOperatorMap)(InitializerContext* context) { {"alwaysFalse", &parseAlwaysBoolean<AlwaysFalseMatchExpression>}, {"alwaysTrue", &parseAlwaysBoolean<AlwaysTrueMatchExpression>}, {"and", &parseTreeTopLevel<AndMatchExpression>}, + {"atomic", &parseAtomicOrIsolated}, {"comment", &parseComment}, {"db", &parseDBRef}, {"expr", &parseExpr}, {"id", &parseDBRef}, + {"isolated", &parseAtomicOrIsolated}, {"jsonSchema", &parseJSONSchema}, {"nor", &parseTreeTopLevel<NorMatchExpression>}, {"or", &parseTreeTopLevel<OrMatchExpression>}, diff --git a/src/mongo/db/matcher/expression_parser.h b/src/mongo/db/matcher/expression_parser.h index 0c9161a3541..2a72cc6309f 100644 --- a/src/mongo/db/matcher/expression_parser.h +++ b/src/mongo/db/matcher/expression_parser.h @@ -97,6 +97,7 @@ public: kJavascript = 1 << 2, kExpr = 1 << 3, kJSONSchema = 1 << 4, + kIsolated = 1 << 5, }; using AllowedFeatureSet = unsigned long long; static constexpr AllowedFeatureSet kBanAllSpecialFeatures = 0; diff --git a/src/mongo/db/matcher/expression_parser_test.cpp b/src/mongo/db/matcher/expression_parser_test.cpp index d2bcd91a1b4..4343cf60403 100644 --- a/src/mongo/db/matcher/expression_parser_test.cpp +++ b/src/mongo/db/matcher/expression_parser_test.cpp @@ -65,6 +65,56 @@ TEST(MatchExpressionParserTest, Multiple1) { ASSERT(!result.getValue()->matchesBSON(BSON("x" << 5 << "y" << 4))); } +TEST(AtomicMatchExpressionTest, AtomicOperatorSuccessfullyParsesWhenFeatureBitIsSet) { + auto query = BSON("x" << 5 << "$atomic" << 1); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + auto result = MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kIsolated); + ASSERT_OK(result.getStatus()); +} + +TEST(AtomicMatchExpressionTest, AtomicOperatorFailsToParseIfNotTopLevel) { + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + auto query = BSON("x" << 5 << "y" << BSON("$atomic" << 1)); + auto result = MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kIsolated); + ASSERT_NOT_OK(result.getStatus()); + ASSERT_EQ(ErrorCodes::BadValue, result.getStatus()); +} + +TEST(AtomicMatchExpressionTest, AtomicOperatorFailsToParseIfFeatureBitIsNotSet) { + auto query = BSON("x" << 5 << "$atomic" << 1); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + auto result = MatchExpressionParser::parse(query, expCtx); + ASSERT_EQ(ErrorCodes::QueryFeatureNotAllowed, result.getStatus()); +} + +TEST(IsolatedMatchExpressionTest, IsolatedOperatorSuccessfullyParsesWhenFeatureBitIsSet) { + auto query = BSON("x" << 5 << "$isolated" << 1); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + auto result = MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kIsolated); + ASSERT_OK(result.getStatus()); +} + +TEST(IsolatedMatchExpressionTest, IsolatedOperatorFailsToParseIfFeatureBitIsNotSet) { + // Query parsing fails if $isolated is not in the allowed feature set. + auto query = BSON("x" << 5 << "$isolated" << 1); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + auto result = MatchExpressionParser::parse(query, expCtx); + ASSERT_NOT_OK(result.getStatus()); + ASSERT_EQ(ErrorCodes::QueryFeatureNotAllowed, result.getStatus()); +} + +TEST(IsolatedMatchExpressionTest, IsolatedOperatorFailsToParseIfNotTopLevel) { + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + auto query = BSON("x" << 5 << "y" << BSON("$isolated" << 1)); + auto result = MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kIsolated); + ASSERT_NOT_OK(result.getStatus()); + ASSERT_EQ(ErrorCodes::BadValue, result.getStatus()); +} + TEST(MatchExpressionParserTest, MinDistanceWithoutNearFailsToParse) { BSONObj query = fromjson("{loc: {$minDistance: 10}}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); diff --git a/src/mongo/db/ops/parsed_delete.cpp b/src/mongo/db/ops/parsed_delete.cpp index 227e62c098b..a213d038ad9 100644 --- a/src/mongo/db/ops/parsed_delete.cpp +++ b/src/mongo/db/ops/parsed_delete.cpp @@ -109,7 +109,18 @@ const DeleteRequest* ParsedDelete::getRequest() const { } PlanExecutor::YieldPolicy ParsedDelete::yieldPolicy() const { - return _request->isGod() ? PlanExecutor::NO_YIELD : _request->getYieldPolicy(); + if (_request->isGod()) { + return PlanExecutor::NO_YIELD; + } + if (_request->getYieldPolicy() == PlanExecutor::YIELD_AUTO && isIsolated()) { + return PlanExecutor::WRITE_CONFLICT_RETRY_ONLY; // Don't yield locks. + } + return _request->getYieldPolicy(); +} + +bool ParsedDelete::isIsolated() const { + return _canonicalQuery.get() ? _canonicalQuery->isIsolated() + : QueryRequest::isQueryIsolated(_request->getQuery()); } bool ParsedDelete::hasParsedQuery() const { diff --git a/src/mongo/db/ops/parsed_delete.h b/src/mongo/db/ops/parsed_delete.h index 7b4856c91b7..9cda64718a1 100644 --- a/src/mongo/db/ops/parsed_delete.h +++ b/src/mongo/db/ops/parsed_delete.h @@ -84,11 +84,16 @@ public: const DeleteRequest* getRequest() const; /** - * Get the YieldPolicy, adjusted for GodMode. + * Get the YieldPolicy, adjusted for $isolated and GodMode. */ PlanExecutor::YieldPolicy yieldPolicy() const; /** + * Is this update supposed to be isolated? + */ + bool isIsolated() const; + + /** * As an optimization, we don't create a canonical query for updates with simple _id * queries. Use this method to determine whether or not we actually parsed the query. */ diff --git a/src/mongo/db/ops/parsed_update.cpp b/src/mongo/db/ops/parsed_update.cpp index d7a31d0b936..4a463731c68 100644 --- a/src/mongo/db/ops/parsed_update.cpp +++ b/src/mongo/db/ops/parsed_update.cpp @@ -186,7 +186,18 @@ Status ParsedUpdate::parseArrayFilters() { } PlanExecutor::YieldPolicy ParsedUpdate::yieldPolicy() const { - return _request->isGod() ? PlanExecutor::NO_YIELD : _request->getYieldPolicy(); + if (_request->isGod()) { + return PlanExecutor::NO_YIELD; + } + if (_request->getYieldPolicy() == PlanExecutor::YIELD_AUTO && isIsolated()) { + return PlanExecutor::WRITE_CONFLICT_RETRY_ONLY; // Don't yield locks. + } + return _request->getYieldPolicy(); +} + +bool ParsedUpdate::isIsolated() const { + return _canonicalQuery.get() ? _canonicalQuery->isIsolated() + : QueryRequest::isQueryIsolated(_request->getQuery()); } bool ParsedUpdate::hasParsedQuery() const { diff --git a/src/mongo/db/ops/parsed_update.h b/src/mongo/db/ops/parsed_update.h index 8e006f461d1..5510b6e7dde 100644 --- a/src/mongo/db/ops/parsed_update.h +++ b/src/mongo/db/ops/parsed_update.h @@ -92,11 +92,16 @@ public: UpdateDriver* getDriver(); /** - * Get the YieldPolicy, adjusted for GodMode. + * Get the YieldPolicy, adjusted for $isolated and GodMode. */ PlanExecutor::YieldPolicy yieldPolicy() const; /** + * Is this update supposed to be isolated? + */ + bool isIsolated() const; + + /** * As an optimization, we don't create a canonical query for updates with simple _id * queries. Use this method to determine whether or not we actually parsed the query. */ diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp index 146c5a40cf8..0e761e36290 100644 --- a/src/mongo/db/ops/write_ops_exec.cpp +++ b/src/mongo/db/ops/write_ops_exec.cpp @@ -587,10 +587,10 @@ static SingleWriteResult performSingleUpdateOp(OperationContext* opCtx, request.setUpsert(op.getUpsert()); auto readConcernArgs = repl::ReadConcernArgs::get(opCtx); - request.setYieldPolicy(readConcernArgs.getLevel() == - repl::ReadConcernLevel::kSnapshotReadConcern - ? PlanExecutor::INTERRUPT_ONLY - : PlanExecutor::YIELD_AUTO); + request.setYieldPolicy( + readConcernArgs.getLevel() == repl::ReadConcernLevel::kSnapshotReadConcern + ? PlanExecutor::INTERRUPT_ONLY + : PlanExecutor::YIELD_AUTO); // ParsedUpdate overrides this for $isolated. ParsedUpdate parsedUpdate(opCtx, &request); uassertStatusOK(parsedUpdate.parseRequest()); @@ -605,7 +605,7 @@ static SingleWriteResult performSingleUpdateOp(OperationContext* opCtx, collection.emplace(opCtx, ns, MODE_IX, // DB is always IX, even if collection is X. - MODE_IX); + parsedUpdate.isIsolated() ? MODE_X : MODE_IX); if (collection->getCollection() || !op.getUpsert()) break; @@ -742,10 +742,10 @@ static SingleWriteResult performSingleDeleteOp(OperationContext* opCtx, request.setCollation(write_ops::collationOf(op)); request.setMulti(op.getMulti()); auto readConcernArgs = repl::ReadConcernArgs::get(opCtx); - request.setYieldPolicy(readConcernArgs.getLevel() == - repl::ReadConcernLevel::kSnapshotReadConcern - ? PlanExecutor::INTERRUPT_ONLY - : PlanExecutor::YIELD_AUTO); + request.setYieldPolicy( + readConcernArgs.getLevel() == repl::ReadConcernLevel::kSnapshotReadConcern + ? PlanExecutor::INTERRUPT_ONLY + : PlanExecutor::YIELD_AUTO); // ParsedDelete overrides this for $isolated. request.setStmtId(stmtId); ParsedDelete parsedDelete(opCtx, &request); @@ -760,7 +760,7 @@ static SingleWriteResult performSingleDeleteOp(OperationContext* opCtx, AutoGetCollection collection(opCtx, ns, MODE_IX, // DB is always IX, even if collection is X. - MODE_IX); + parsedDelete.isIsolated() ? MODE_X : MODE_IX); if (collection.getDb()) { curOp.raiseDbProfileLevel(collection.getDb()->getProfilingLevel()); } diff --git a/src/mongo/db/query/canonical_query.cpp b/src/mongo/db/query/canonical_query.cpp index b2d623fe698..d498367fd50 100644 --- a/src/mongo/db/query/canonical_query.cpp +++ b/src/mongo/db/query/canonical_query.cpp @@ -221,6 +221,13 @@ Status CanonicalQuery::init(OperationContext* opCtx, _collator = std::move(collator); _canHaveNoopMatchNodes = canHaveNoopMatchNodes; + _isIsolated = QueryRequest::isQueryIsolated(_qr->getFilter()); + if (_isIsolated) { + RARELY { + warning() << "The $isolated/$atomic option is deprecated. See " + "http://dochub.mongodb.org/core/isolated-deprecation"; + } + } // Normalize, sort and validate tree. _root = MatchExpression::optimize(std::move(root)); @@ -278,7 +285,11 @@ bool CanonicalQuery::isSimpleIdQuery(const BSONObj& query) { // But it can be BinData. return false; } + } else if (elt.fieldName()[0] == '$' && (str::equals("$isolated", elt.fieldName()) || + str::equals("$atomic", elt.fieldName()))) { + // ok, passthrough } else { + // If the field is not _id, it must be $isolated/$atomic. return false; } } diff --git a/src/mongo/db/query/canonical_query.h b/src/mongo/db/query/canonical_query.h index 921b4eadb92..83fd713512a 100644 --- a/src/mongo/db/query/canonical_query.h +++ b/src/mongo/db/query/canonical_query.h @@ -92,7 +92,8 @@ public: MatchExpression* root); /** - * Returns true if "query" describes an exact-match query on _id. + * Returns true if "query" describes an exact-match query on _id, possibly with + * the $isolated/$atomic modifier. */ static bool isSimpleIdQuery(const BSONObj& query); @@ -170,6 +171,14 @@ public: return _canHaveNoopMatchNodes; } + /** + * Returns true if the query this CanonicalQuery was parsed from included a $isolated/$atomic + * operator. + */ + bool isIsolated() const { + return _isIsolated; + } + private: // You must go through canonicalize to create a CanonicalQuery. CanonicalQuery() {} @@ -190,6 +199,8 @@ private: std::unique_ptr<CollatorInterface> _collator; bool _canHaveNoopMatchNodes = false; + + bool _isIsolated; }; } // namespace mongo diff --git a/src/mongo/db/query/canonical_query_test.cpp b/src/mongo/db/query/canonical_query_test.cpp index 8f7a35ad8ac..0aa386b9800 100644 --- a/src/mongo/db/query/canonical_query_test.cpp +++ b/src/mongo/db/query/canonical_query_test.cpp @@ -207,6 +207,37 @@ std::unique_ptr<CanonicalQuery> canonicalize(const char* queryStr, } /** + * Test that CanonicalQuery::isIsolated() returns correctly. + */ +TEST(CanonicalQueryTest, IsIsolatedReturnsTrueWithIsolated) { + unique_ptr<CanonicalQuery> cq = canonicalize("{$isolated: 1, x: 3}", + MatchExpressionParser::kDefaultSpecialFeatures | + MatchExpressionParser::kIsolated); + ASSERT_TRUE(cq->isIsolated()); +} + +TEST(CanonicalQueryTest, IsIsolatedReturnsTrueWithAtomic) { + unique_ptr<CanonicalQuery> cq = canonicalize("{$atomic: 1, x: 3}", + MatchExpressionParser::kDefaultSpecialFeatures | + MatchExpressionParser::kIsolated); + ASSERT_TRUE(cq->isIsolated()); +} + +TEST(CanonicalQueryTest, IsIsolatedReturnsFalseWithIsolated) { + unique_ptr<CanonicalQuery> cq = canonicalize("{$isolated: 0, x: 3}", + MatchExpressionParser::kDefaultSpecialFeatures | + MatchExpressionParser::kIsolated); + ASSERT_FALSE(cq->isIsolated()); +} + +TEST(CanonicalQueryTest, IsIsolatedReturnsFalseWithAtomic) { + unique_ptr<CanonicalQuery> cq = canonicalize("{$atomic: 0, x: 3}", + MatchExpressionParser::kDefaultSpecialFeatures | + MatchExpressionParser::kIsolated); + ASSERT_FALSE(cq->isIsolated()); +} + +/** * Test function for CanonicalQuery::normalize. */ void testNormalizeQuery(const char* queryStr, const char* expectedExprStr) { diff --git a/src/mongo/db/query/find.cpp b/src/mongo/db/query/find.cpp index ebe4c2937c3..43d86c1beab 100644 --- a/src/mongo/db/query/find.cpp +++ b/src/mongo/db/query/find.cpp @@ -532,7 +532,8 @@ std::string runQuery(OperationContext* opCtx, q, expCtx, ExtensionsCallbackReal(opCtx, &nss), - MatchExpressionParser::kAllowAllSpecialFeatures); + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); if (!statusWithCQ.isOK()) { uasserted(17287, str::stream() << "Can't canonicalize query: " diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index ce5e2b3718e..a645c9105d9 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -1288,7 +1288,8 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorCount( collection ? static_cast<const ExtensionsCallback&>( ExtensionsCallbackReal(opCtx, &collection->ns())) : static_cast<const ExtensionsCallback&>(ExtensionsCallbackNoop()), - MatchExpressionParser::kAllowAllSpecialFeatures); + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); @@ -1557,7 +1558,8 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDistinct( std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures); + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); } diff --git a/src/mongo/db/query/parsed_distinct.cpp b/src/mongo/db/query/parsed_distinct.cpp index aa6bfd71177..8ff2277bee6 100644 --- a/src/mongo/db/query/parsed_distinct.cpp +++ b/src/mongo/db/query/parsed_distinct.cpp @@ -207,7 +207,8 @@ StatusWith<ParsedDistinct> ParsedDistinct::parse(OperationContext* opCtx, std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures); + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kIsolated); if (!cq.isOK()) { return cq.getStatus(); } diff --git a/src/mongo/db/query/query_request.cpp b/src/mongo/db/query/query_request.cpp index 4978aad81f8..16c453f7f2e 100644 --- a/src/mongo/db/query/query_request.cpp +++ b/src/mongo/db/query/query_request.cpp @@ -714,6 +714,19 @@ bool QueryRequest::isValidSortOrder(const BSONObj& sortObj) { return true; } +// static +bool QueryRequest::isQueryIsolated(const BSONObj& query) { + BSONObjIterator iter(query); + while (iter.more()) { + BSONElement elt = iter.next(); + if (str::equals(elt.fieldName(), "$isolated") && elt.trueValue()) + return true; + if (str::equals(elt.fieldName(), "$atomic") && elt.trueValue()) + return true; + } + return false; +} + // // Old QueryRequest parsing code: SOON TO BE DEPRECATED. // diff --git a/src/mongo/db/query/query_request.h b/src/mongo/db/query/query_request.h index 7a89e466faf..1bca2d3daad 100644 --- a/src/mongo/db/query/query_request.h +++ b/src/mongo/db/query/query_request.h @@ -115,6 +115,12 @@ public: */ static bool isValidSortOrder(const BSONObj& sortObj); + /** + * Returns true if the query described by "query" should execute + * at an elevated level of isolation (i.e., $isolated was specified). + */ + static bool isQueryIsolated(const BSONObj& query); + // Read preference is attached to commands in "wrapped" form, e.g. // { $query: { <cmd>: ... } , <kWrappedReadPrefField>: { ... } } // diff --git a/src/mongo/db/query/query_request_test.cpp b/src/mongo/db/query/query_request_test.cpp index edcf58cecb1..2192960b859 100644 --- a/src/mongo/db/query/query_request_test.cpp +++ b/src/mongo/db/query/query_request_test.cpp @@ -240,6 +240,22 @@ TEST(QueryRequestTest, AllowTailableWithNaturalSort) { ASSERT_BSONOBJ_EQ(result.getValue()->getSort(), BSON("$natural" << 1)); } +TEST(QueryRequestTest, IsIsolatedReturnsTrueWithIsolated) { + ASSERT_TRUE(QueryRequest::isQueryIsolated(BSON("$isolated" << 1))); +} + +TEST(QueryRequestTest, IsIsolatedReturnsTrueWithAtomic) { + ASSERT_TRUE(QueryRequest::isQueryIsolated(BSON("$atomic" << 1))); +} + +TEST(QueryRequestTest, IsIsolatedReturnsFalseWithIsolated) { + ASSERT_FALSE(QueryRequest::isQueryIsolated(BSON("$isolated" << false))); +} + +TEST(QueryRequestTest, IsIsolatedReturnsFalseWithAtomic) { + ASSERT_FALSE(QueryRequest::isQueryIsolated(BSON("$atomic" << false))); +} + // // Test compatibility of various projection and sort objects. // diff --git a/src/mongo/db/storage/mmap_v1/dur.h b/src/mongo/db/storage/mmap_v1/dur.h index 06b38255c25..b505de833f6 100644 --- a/src/mongo/db/storage/mmap_v1/dur.h +++ b/src/mongo/db/storage/mmap_v1/dur.h @@ -91,8 +91,9 @@ public: /** Commit if enough bytes have been modified. Current threshold is 50MB The idea is that long running write operations that don't yield - (like creating an index) can call this whenever the db is in a sane state and it will - prevent commits from growing too large. + (like creating an index or update with $atomic) can call this + whenever the db is in a sane state and it will prevent commits + from growing too large. @return true if commited */ virtual bool commitIfNeeded() = 0; |