[SERVER-57483] Results from $lookup stage are not cached for a resharding operation's sub-pipeline Created: 07/Jun/21  Updated: 29/Oct/23  Resolved: 10/Jun/21

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 5.0.0-rc2, 5.1.0-rc0

Type: Bug Priority: Critical - P2
Reporter: Max Hirschhorn Assignee: Max Hirschhorn
Resolution: Fixed Votes: 0
Labels: PM-234-M3, PM-234-T-data-clone, post-rc0
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Depends
Problem/Incident
is caused by SERVER-45538 Implement and test shard version retr... Closed
Related
related to SERVER-57579 DocumentSourceSequentialCache doesn't... Closed
related to SERVER-57667 Improve processing speed for reshardi... Closed
is related to SERVER-57168 pipeline->clone() breaks on pipelines... Closed
is related to SERVER-30399 Add caching for $lookup non-correlate... Closed
is related to SERVER-57668 Cache chunk bounds as an array in res... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Backport Requested:
v5.0
Sprint: Sharding 2021-06-14
Participants:
Story Points: 1

 Description   

DocumentSourceLookUp::buildPipeline() calls ShardServerProcessInterface::attachCursorSourceToPipeline() which calls sharded_agg_helpers::attachCursorToPipeline() and always serializes and re-parses, even when the pipeline may execute locally on the shard via ShardServerProcessInterface::attachCursorSourceToPipelineForLocalRead().

Because the $sequentialCache stage omits itself from the serialized pipeline, the parsed pipeline which executes locally on the shard won't ever use the cache.



 Comments   
Comment by Vivian Ge (Inactive) [ 06/Oct/21 ]

Updating the fixversion since branching activities occurred yesterday. This ticket will be in rc0 when it’s been triggered. For more active release information, please keep an eye on #server-release. Thank you!

Comment by Max Hirschhorn [ 15/Jun/21 ]

Note for posterity: ted.tuckman, pointed out that the $match stages in the ReshardingCollectionCloner's $lookup pipeline are being coalesced from the call to Pipeline::optimizePipeline() and causing the $sequentialCache to be removed (SERVER-57705). I hadn't noticed this in my performance testing because I had also been including my changes for SERVER-57668 along with my changes for SERVER-57483. The projection stage between the two $match stages prevents the $match stages from being coalesced and therefore preserves the $sequentialCache stage.

Comment by Githook User [ 10/Jun/21 ]

Author:

{'name': 'Max Hirschhorn', 'email': 'max.hirschhorn@mongodb.com', 'username': 'visemet'}

Message: SERVER-57483 Fix caching $lookup results for config.cache.chunks.

(cherry picked from commit 0c046b785bd8d48114f627e105c7215e4734c6db)
Branch: v5.0
https://github.com/mongodb/mongo/commit/4da58947c643ada0596061ceeb8d1a63b6faeb0d

Comment by Githook User [ 10/Jun/21 ]

Author:

{'name': 'Max Hirschhorn', 'email': 'max.hirschhorn@mongodb.com', 'username': 'visemet'}

Message: SERVER-57483 Fix caching $lookup results for config.cache.chunks.
Branch: master
https://github.com/mongodb/mongo/commit/0c046b785bd8d48114f627e105c7215e4734c6db

Comment by Charlie Swanson [ 09/Jun/21 ]

max.hirschhorn we discussed you sending us a CR for a patch which is similar to what you outlined in your comment, so I'm assigning this to you. I filed SERVER-57579 for the follow-up work that we can handle as a more permanent fix.

Comment by Max Hirschhorn [ 07/Jun/21 ]

One solution could be to avoid calling Pipeline::clone() when the executes locally on the shard. This wouldn't help when the server is running with --setParameter internalQueryAllowShardedLookup=true but that isn't an officially supported configuration anyhow.

diff --git a/src/mongo/db/pipeline/sharded_agg_helpers.cpp b/src/mongo/db/pipeline/sharded_agg_helpers.cpp
index cd5d35f992..b3723c1b0d 100644
--- a/src/mongo/db/pipeline/sharded_agg_helpers.cpp
+++ b/src/mongo/db/pipeline/sharded_agg_helpers.cpp
@@ -1261,16 +1261,19 @@ std::unique_ptr<Pipeline, PipelineDeleter> attachCursorToPipeline(Pipeline* owne
     invariant(pipeline->getSources().empty() ||
               !dynamic_cast<DocumentSourceMergeCursors*>(pipeline->getSources().front().get()));
 
+    if (!allowTargetingShards || expCtx->ns.isLocal() || expCtx->ns.isConfigDotCacheDotChunks()) {
+        // Use 'ownedPipeline' rather than a serialized and re-parsed copy of it when reading from
+        // the collection locally. This is important because optimizations such as the
+        // $sequentialCache stage are lost when the pipeline is serialized.
+        return expCtx->mongoProcessInterface->attachCursorSourceToPipelineForLocalRead(
+            pipeline.release());
+    }
+
     auto catalogCache = Grid::get(expCtx->opCtx)->catalogCache();
     return shardVersionRetry(
         expCtx->opCtx, catalogCache, expCtx->ns, "targeting pipeline to attach cursors"_sd, [&]() {
             auto pipelineToTarget = pipeline->clone();
-            if (allowTargetingShards && !expCtx->ns.isConfigDotCacheDotChunks() &&
-                expCtx->ns.db() != "local") {
-                return targetShardsAndAddMergeCursors(expCtx, std::move(pipelineToTarget));
-            }
-            return expCtx->mongoProcessInterface->attachCursorSourceToPipelineForLocalRead(
-                pipelineToTarget.release());
+            return targetShardsAndAddMergeCursors(expCtx, std::move(pipelineToTarget));
         });
 }

Generated at Thu Feb 08 05:41:57 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.