From b45d9b1215bedf4328bded55913475dd6b589e92 Mon Sep 17 00:00:00 2001 From: Hugh Tong Date: Mon, 15 May 2023 15:28:20 +0000 Subject: SERVER-76634 Add SerializationContext object to ExpressionContext --- src/mongo/db/curop.cpp | 19 +++++++--- src/mongo/db/curop.h | 6 ++- src/mongo/db/curop_test.cpp | 43 +++++++++++++++++++++- src/mongo/db/operation_context_test.cpp | 7 +++- .../db/pipeline/document_source_coll_stats.cpp | 13 +++++-- src/mongo/db/pipeline/expression_context.cpp | 14 +++++-- src/mongo/db/pipeline/expression_context.h | 5 ++- .../db/pipeline/expression_context_for_test.h | 24 ++++++------ .../common_mongod_process_interface.cpp | 4 +- .../common_mongod_process_interface.h | 2 +- .../process_interface/common_process_interface.cpp | 15 ++++++-- .../process_interface/common_process_interface.h | 2 +- .../process_interface/mongos_process_interface.cpp | 4 +- .../process_interface/mongos_process_interface.h | 2 +- 14 files changed, 119 insertions(+), 41 deletions(-) diff --git a/src/mongo/db/curop.cpp b/src/mongo/db/curop.cpp index c24057452af..49fd43c0035 100644 --- a/src/mongo/db/curop.cpp +++ b/src/mongo/db/curop.cpp @@ -183,7 +183,7 @@ CurOp* CurOp::get(const OperationContext& opCtx) { return _curopStack(opCtx).top(); } -void CurOp::reportCurrentOpForClient(OperationContext* opCtx, +void CurOp::reportCurrentOpForClient(const boost::intrusive_ptr& expCtx, Client* client, bool truncateOps, bool backtraceMode, @@ -210,8 +210,9 @@ void CurOp::reportCurrentOpForClient(OperationContext* opCtx, // Fill out the rest of the BSONObj with opCtx specific details. infoBuilder->appendBool("active", client->hasAnyActiveCurrentOp()); - infoBuilder->append("currentOpTime", - opCtx->getServiceContext()->getPreciseClockSource()->now().toString()); + infoBuilder->append( + "currentOpTime", + expCtx->opCtx->getServiceContext()->getPreciseClockSource()->now().toString()); auto authSession = AuthorizationSession::get(client); // Depending on whether the authenticated user is the same user which ran the command, @@ -261,7 +262,11 @@ void CurOp::reportCurrentOpForClient(OperationContext* opCtx, lsid->serialize(&lsidBuilder); } - CurOp::get(clientOpCtx)->reportState(infoBuilder, truncateOps); + tassert(7663403, + str::stream() << "SerializationContext on the expCtx should not be empty, with ns: " + << expCtx->ns.ns(), + expCtx->serializationCtxt != SerializationContext::stateDefault()); + CurOp::get(clientOpCtx)->reportState(infoBuilder, expCtx->serializationCtxt, truncateOps); } #ifndef MONGO_CONFIG_USE_RAW_LATCHES @@ -705,7 +710,9 @@ BSONObj CurOp::truncateAndSerializeGenericCursor(GenericCursor* cursor, return serialized; } -void CurOp::reportState(BSONObjBuilder* builder, bool truncateOps) { +void CurOp::reportState(BSONObjBuilder* builder, + const SerializationContext& serializationContext, + bool truncateOps) { auto opCtx = this->opCtx(); auto start = _start.load(); if (start) { @@ -716,7 +723,7 @@ void CurOp::reportState(BSONObjBuilder* builder, bool truncateOps) { } builder->append("op", logicalOpToString(_logicalOp)); - builder->append("ns", NamespaceStringUtil::serialize(_nss)); + builder->append("ns", NamespaceStringUtil::serialize(_nss, serializationContext)); bool omitAndRedactInformation = CurOp::get(opCtx)->debug().shouldOmitDiagnosticInformation; builder->append("redacted", omitAndRedactInformation); diff --git a/src/mongo/db/curop.h b/src/mongo/db/curop.h index 352b10961b0..8851993b015 100644 --- a/src/mongo/db/curop.h +++ b/src/mongo/db/curop.h @@ -432,7 +432,7 @@ public: * report, since this may be called in either a mongoD or mongoS context and the latter does not * supply lock stats. The client must be locked before calling this method. */ - static void reportCurrentOpForClient(OperationContext* opCtx, + static void reportCurrentOpForClient(const boost::intrusive_ptr& expCtx, Client* client, bool truncateOps, bool backtraceMode, @@ -875,7 +875,9 @@ public: * If called from a thread other than the one executing the operation associated with this * CurOp, it is necessary to lock the associated Client object before executing this method. */ - void reportState(BSONObjBuilder* builder, bool truncateOps = false); + void reportState(BSONObjBuilder* builder, + const SerializationContext& serializationContext, + bool truncateOps = false); /** * Sets the message for FailPoints used. diff --git a/src/mongo/db/curop_test.cpp b/src/mongo/db/curop_test.cpp index c5d2d7947ca..8a737f1a748 100644 --- a/src/mongo/db/curop_test.cpp +++ b/src/mongo/db/curop_test.cpp @@ -32,6 +32,7 @@ #include "mongo/bson/bsonobjbuilder.h" #include "mongo/db/curop.h" #include "mongo/db/query/query_test_service_context.h" +#include "mongo/idl/server_parameter_test_util.h" #include "mongo/unittest/unittest.h" #include "mongo/util/tick_source_mock.h" @@ -269,7 +270,7 @@ TEST(CurOpTest, ShouldNotReportFailpointMsgIfNotSet) { BSONObjBuilder reportedStateWithoutFailpointMsg; { stdx::lock_guard lk(*opCtx->getClient()); - curop->reportState(&reportedStateWithoutFailpointMsg); + curop->reportState(&reportedStateWithoutFailpointMsg, SerializationContext()); } auto bsonObj = reportedStateWithoutFailpointMsg.done(); @@ -304,5 +305,45 @@ TEST(CurOpTest, ElapsedTimeReflectsTickSource) { ASSERT_EQ(Milliseconds{20}, duration_cast(curop->elapsedTimeTotal())); } +TEST(CurOpTest, CheckNSAgainstSerializationContext) { + RAIIServerParameterControllerForTest multitenanyController("multitenancySupport", true); + TenantId tid = TenantId(OID::gen()); + + QueryTestServiceContext serviceContext; + auto opCtx = serviceContext.makeOperationContext(); + + auto curop = CurOp::get(*opCtx); + + // Create dummy command. + BSONObj command = BSON("a" << 3); + + // Set dummy 'ns' and 'command'. + curop->setGenericOpRequestDetails( + NamespaceString::createNamespaceString_forTest(tid, "testDb.coll"), + nullptr, + command, + NetworkOp::dbQuery); + + // Test without using the expectPrefix field. + // TODO SERVER-74284: uncomment the below and remove the line after when command + // serializer/deserializer is plumbed in + // for (bool tenantIdFromDollarTenantOrSecurityToken : {false, true}) { + bool tenantIdFromDollarTenantOrSecurityToken = false; + + SerializationContext sc = SerializationContext::stateCommandRequest(); + sc.setTenantIdSource(tenantIdFromDollarTenantOrSecurityToken); + + BSONObjBuilder builder; + { + stdx::lock_guard lk(*opCtx->getClient()); + curop->reportState(&builder, sc); + } + auto bsonObj = builder.done(); + + std::string serializedNs = + tenantIdFromDollarTenantOrSecurityToken ? "testDb.coll" : tid.toString() + "_testDb.coll"; + ASSERT_EQ(serializedNs, bsonObj.getField("ns").String()); + // } +} } // namespace } // namespace mongo diff --git a/src/mongo/db/operation_context_test.cpp b/src/mongo/db/operation_context_test.cpp index 2f1d4e183b3..11205ad7a19 100644 --- a/src/mongo/db/operation_context_test.cpp +++ b/src/mongo/db/operation_context_test.cpp @@ -1116,6 +1116,9 @@ TEST_F(OperationContextTest, CurrentOpExcludesKilledOperations) { auto client = makeClient("MainClient"); auto opCtx = client->makeOperationContext(); + const boost::intrusive_ptr expCtx(new ExpressionContext( + opCtx.get(), nullptr, NamespaceString::createNamespaceString_forTest("foo.bar"_sd))); + for (auto truncateOps : {true, false}) { for (auto backtraceMode : {true, false}) { BSONObjBuilder bobNoOpCtx, bobKilledOpCtx; @@ -1129,14 +1132,14 @@ TEST_F(OperationContextTest, CurrentOpExcludesKilledOperations) { // Generate report in absence of any opCtx CurOp::reportCurrentOpForClient( - opCtx.get(), threadClient.get(), truncateOps, backtraceMode, &bobNoOpCtx); + expCtx, threadClient.get(), truncateOps, backtraceMode, &bobNoOpCtx); auto threadOpCtx = threadClient->makeOperationContext(); getServiceContext()->killAndDelistOperation(threadOpCtx.get()); // Generate report in presence of a killed opCtx CurOp::reportCurrentOpForClient( - opCtx.get(), threadClient.get(), truncateOps, backtraceMode, &bobKilledOpCtx); + expCtx, threadClient.get(), truncateOps, backtraceMode, &bobKilledOpCtx); }); thread.join(); diff --git a/src/mongo/db/pipeline/document_source_coll_stats.cpp b/src/mongo/db/pipeline/document_source_coll_stats.cpp index 1df5e681484..12413358639 100644 --- a/src/mongo/db/pipeline/document_source_coll_stats.cpp +++ b/src/mongo/db/pipeline/document_source_coll_stats.cpp @@ -65,8 +65,15 @@ intrusive_ptr DocumentSourceCollStats::createFromBson( uassert(40166, str::stream() << "$collStats must take a nested object but found: " << specElem, specElem.type() == BSONType::Object); - auto spec = - DocumentSourceCollStatsSpec::parse(IDLParserContext(kStageName), specElem.embeddedObject()); + + // TODO SERVER-77056: add assertion to validate pExpCtx->serializationCtxt != stateDefault() + + auto spec = DocumentSourceCollStatsSpec::parse( + IDLParserContext(kStageName, + false /* apiStrict */, + pExpCtx->ns.tenantId(), + SerializationContext::stateCommandReply(pExpCtx->serializationCtxt)), + specElem.embeddedObject()); return make_intrusive(pExpCtx, std::move(spec)); } @@ -79,7 +86,7 @@ BSONObj DocumentSourceCollStats::makeStatsForNs( BSONObjBuilder builder; // We need to use the serialization context from the request when calling - // NamespaceStringUtil to build the reply + // NamespaceStringUtil to build the reply. builder.append( "ns", NamespaceStringUtil::serialize( diff --git a/src/mongo/db/pipeline/expression_context.cpp b/src/mongo/db/pipeline/expression_context.cpp index c171cca3cda..041e4642717 100644 --- a/src/mongo/db/pipeline/expression_context.cpp +++ b/src/mongo/db/pipeline/expression_context.cpp @@ -77,7 +77,8 @@ ExpressionContext::ExpressionContext(OperationContext* opCtx, {}, // resolvedNamespaces findCmd.getNamespaceOrUUID().uuid(), findCmd.getLet(), - mayDbProfile) {} + mayDbProfile, + findCmd.getSerializationContext()) {} ExpressionContext::ExpressionContext(OperationContext* opCtx, const AggregateCommandRequest& request, @@ -101,7 +102,8 @@ ExpressionContext::ExpressionContext(OperationContext* opCtx, std::move(resolvedNamespaces), std::move(collUUID), request.getLet(), - mayDbProfile) { + mayDbProfile, + request.getSerializationContext()) { if (request.getIsMapReduceCommand()) { // mapReduce command JavaScript invocation is only subject to the server global @@ -126,7 +128,8 @@ ExpressionContext::ExpressionContext( StringMap resolvedNamespaces, boost::optional collUUID, const boost::optional& letParameters, - bool mayDbProfile) + bool mayDbProfile, + const SerializationContext& serializationCtx) : explain(explain), fromMongos(fromMongos), needsMerge(needsMerge), @@ -134,6 +137,7 @@ ExpressionContext::ExpressionContext( !(opCtx && opCtx->readOnly())), // Disallow disk use if in read-only mode. bypassDocumentValidation(bypassDocumentValidation), ns(ns), + serializationCtxt(serializationCtx), uuid(std::move(collUUID)), opCtx(opCtx), mongoProcessInterface(mongoProcessInterface), @@ -242,7 +246,8 @@ boost::intrusive_ptr ExpressionContext::copyWith( _resolvedNamespaces, uuid, boost::none /* letParameters */, - mayDbProfile); + mayDbProfile, + SerializationContext()); expCtx->inMongos = inMongos; expCtx->maxFeatureCompatibilityVersion = maxFeatureCompatibilityVersion; @@ -263,6 +268,7 @@ boost::intrusive_ptr ExpressionContext::copyWith( expCtx->originalAggregateCommand = originalAggregateCommand.getOwned(); expCtx->inLookup = inLookup; + expCtx->serializationCtxt = serializationCtxt; // Note that we intentionally skip copying the value of '_interruptCounter' because 'expCtx' is // intended to be used for executing a separate aggregation pipeline. diff --git a/src/mongo/db/pipeline/expression_context.h b/src/mongo/db/pipeline/expression_context.h index abef5b952da..3a0c3b2c2cb 100644 --- a/src/mongo/db/pipeline/expression_context.h +++ b/src/mongo/db/pipeline/expression_context.h @@ -168,7 +168,8 @@ public: StringMap resolvedNamespaces, boost::optional collUUID, const boost::optional& letParameters = boost::none, - bool mayDbProfile = true); + bool mayDbProfile = true, + const SerializationContext& serializationCtx = SerializationContext()); /** * Constructs an ExpressionContext suitable for use outside of the aggregation system, including @@ -464,6 +465,8 @@ public: NamespaceString ns; + SerializationContext serializationCtxt; + // If known, the UUID of the execution namespace for this aggregation command. boost::optional uuid; diff --git a/src/mongo/db/pipeline/expression_context_for_test.h b/src/mongo/db/pipeline/expression_context_for_test.h index 6cd98445d8f..24ed5c1de43 100644 --- a/src/mongo/db/pipeline/expression_context_for_test.h +++ b/src/mongo/db/pipeline/expression_context_for_test.h @@ -76,11 +76,11 @@ public: LegacyRuntimeConstants(Date_t::now(), Timestamp(1, 0)), {}, // collator std::make_shared(), - {}, // resolvedNamespaces - {}, // collUUID - {}, // let - false // mayDbProfile - ) { + {}, // resolvedNamespaces + {}, // collUUID + {}, // let + false, // mayDbProfile + SerializationContext()) { // If there is an existing global ServiceContext, adopt it. Otherwise, create a new context. // Similarly, we create a new OperationContext or adopt an existing context as appropriate. if (hasGlobalServiceContext()) { @@ -127,11 +127,11 @@ public: LegacyRuntimeConstants(Date_t::now(), Timestamp(1, 0)), {}, // collator std::make_shared(), - {}, // resolvedNamespaces - {}, // collUUID - {}, // let - false // mayDbProfile - ), + {}, // resolvedNamespaces + {}, // collUUID + {}, // let + false, // mayDbProfile + SerializationContext()), _serviceContext(opCtx->getServiceContext()) { // Resolve the TimeZoneDatabase to be used by this ExpressionContextForTest. _setTimeZoneDatabase(); @@ -172,8 +172,8 @@ public: {}, // resolvedNamespaces {}, // collUUID letParameters, - false // mayDbProfile - ), + false, // mayDbProfile + SerializationContext()), _serviceContext(opCtx->getServiceContext()) { // Resolve the TimeZoneDatabase to be used by this ExpressionContextForTest. _setTimeZoneDatabase(); diff --git a/src/mongo/db/pipeline/process_interface/common_mongod_process_interface.cpp b/src/mongo/db/pipeline/process_interface/common_mongod_process_interface.cpp index c3db7716d48..d90d2049c7b 100644 --- a/src/mongo/db/pipeline/process_interface/common_mongod_process_interface.cpp +++ b/src/mongo/db/pipeline/process_interface/common_mongod_process_interface.cpp @@ -645,13 +645,13 @@ bool CommonMongodProcessInterface::fieldsHaveSupportingUniqueIndex( } BSONObj CommonMongodProcessInterface::_reportCurrentOpForClient( - OperationContext* opCtx, + const boost::intrusive_ptr& expCtx, Client* client, CurrentOpTruncateMode truncateOps, CurrentOpBacktraceMode backtraceMode) const { BSONObjBuilder builder; - CurOp::reportCurrentOpForClient(opCtx, + CurOp::reportCurrentOpForClient(expCtx, client, (truncateOps == CurrentOpTruncateMode::kTruncateOps), (backtraceMode == CurrentOpBacktraceMode::kIncludeBacktrace), diff --git a/src/mongo/db/pipeline/process_interface/common_mongod_process_interface.h b/src/mongo/db/pipeline/process_interface/common_mongod_process_interface.h index 07e2d0370ce..0bc59bf7c4f 100644 --- a/src/mongo/db/pipeline/process_interface/common_mongod_process_interface.h +++ b/src/mongo/db/pipeline/process_interface/common_mongod_process_interface.h @@ -145,7 +145,7 @@ protected: const Document& documentKey, MakePipelineOptions opts); - BSONObj _reportCurrentOpForClient(OperationContext* opCtx, + BSONObj _reportCurrentOpForClient(const boost::intrusive_ptr& expCtx, Client* client, CurrentOpTruncateMode truncateOps, CurrentOpBacktraceMode backtraceMode) const final; diff --git a/src/mongo/db/pipeline/process_interface/common_process_interface.cpp b/src/mongo/db/pipeline/process_interface/common_process_interface.cpp index 5cf57474365..ab08e47625a 100644 --- a/src/mongo/db/pipeline/process_interface/common_process_interface.cpp +++ b/src/mongo/db/pipeline/process_interface/common_process_interface.cpp @@ -109,7 +109,7 @@ std::vector CommonProcessInterface::getCurrentOps( } // Delegate to the mongoD- or mongoS-specific implementation of _reportCurrentOpForClient. - ops.emplace_back(_reportCurrentOpForClient(opCtx, client, truncateMode, backtraceMode)); + ops.emplace_back(_reportCurrentOpForClient(expCtx, client, truncateMode, backtraceMode)); } // If 'cursorMode' is set to include idle cursors, retrieve them and add them to ops. @@ -120,8 +120,17 @@ std::vector CommonProcessInterface::getCurrentOps( cursorObj.append("type", "idleCursor"); cursorObj.append("host", getHostNameCachedAndPort()); // First, extract fields which need to go at the top level out of the GenericCursor. - auto ns = cursor.getNs(); - cursorObj.append("ns", ns ? NamespaceStringUtil::serialize(*ns) : ""); + if (auto ns = cursor.getNs()) { + tassert(7663401, + str::stream() + << "SerializationContext on the expCtx should not be empty, with ns: " + << ns->ns(), + expCtx->serializationCtxt != SerializationContext::stateDefault()); + cursorObj.append("ns", + NamespaceStringUtil::serialize(*ns, expCtx->serializationCtxt)); + } else + cursorObj.append("ns", ""); + if (auto lsid = cursor.getLsid()) { cursorObj.append("lsid", lsid->toBSON()); } diff --git a/src/mongo/db/pipeline/process_interface/common_process_interface.h b/src/mongo/db/pipeline/process_interface/common_process_interface.h index 4a4f1d1e990..46981e7dda1 100644 --- a/src/mongo/db/pipeline/process_interface/common_process_interface.h +++ b/src/mongo/db/pipeline/process_interface/common_process_interface.h @@ -154,7 +154,7 @@ protected: * executed by the supplied client. This method is called by the getCurrentOps method of * CommonProcessInterface to delegate to the mongoS- or mongoD- specific implementation. */ - virtual BSONObj _reportCurrentOpForClient(OperationContext* opCtx, + virtual BSONObj _reportCurrentOpForClient(const boost::intrusive_ptr& expCtx, Client* client, CurrentOpTruncateMode truncateOps, CurrentOpBacktraceMode backtraceMode) const = 0; diff --git a/src/mongo/db/pipeline/process_interface/mongos_process_interface.cpp b/src/mongo/db/pipeline/process_interface/mongos_process_interface.cpp index 33e900c06a9..2e804e368d4 100644 --- a/src/mongo/db/pipeline/process_interface/mongos_process_interface.cpp +++ b/src/mongo/db/pipeline/process_interface/mongos_process_interface.cpp @@ -259,13 +259,13 @@ boost::optional MongosProcessInterface::lookupSingleDocumentLocally( } BSONObj MongosProcessInterface::_reportCurrentOpForClient( - OperationContext* opCtx, + const boost::intrusive_ptr& expCtx, Client* client, CurrentOpTruncateMode truncateOps, CurrentOpBacktraceMode backtraceMode) const { BSONObjBuilder builder; - CurOp::reportCurrentOpForClient(opCtx, + CurOp::reportCurrentOpForClient(expCtx, client, (truncateOps == CurrentOpTruncateMode::kTruncateOps), (backtraceMode == CurrentOpBacktraceMode::kIncludeBacktrace), diff --git a/src/mongo/db/pipeline/process_interface/mongos_process_interface.h b/src/mongo/db/pipeline/process_interface/mongos_process_interface.h index 0ea14f6dcc2..ce70e71db25 100644 --- a/src/mongo/db/pipeline/process_interface/mongos_process_interface.h +++ b/src/mongo/db/pipeline/process_interface/mongos_process_interface.h @@ -310,7 +310,7 @@ public: } protected: - BSONObj _reportCurrentOpForClient(OperationContext* opCtx, + BSONObj _reportCurrentOpForClient(const boost::intrusive_ptr& expCtx, Client* client, CurrentOpTruncateMode truncateOps, CurrentOpBacktraceMode backtraceMode) const final; -- cgit v1.2.1