-
Type: Bug
-
Resolution: Fixed
-
Priority: Major - P3
-
Affects Version/s: None
-
Component/s: None
-
Labels:None
-
Fully Compatible
-
ALL
-
QE 2022-04-18
This is easiest to explain with a repro script, so here it is. This should be run against a server where featureFlagSBELookupPushdown is enabled:
(function() { "use strict"; const leftColl = db.left; const rightColl = db.right; leftColl.drop(); rightColl.drop(); assert.commandWorked(leftColl.createIndex({a: 1})); assert.commandWorked(leftColl.createIndex({b: 1})); assert.commandWorked(leftColl.insert([ {_id: 0, a: 1, b: 1}, {_id: 1, a: 1, b: 2}, {_id: 2, a: 1, b: 3}, {_id: 3, a: 1, b: 4}, ])); assert.commandWorked(rightColl.createIndex({c: 1})); for (let i = -30; i < 30; ++i) { assert.commandWorked(rightColl.insert({_id: i, c: i})); } function runQuery() { assert.eq( [{_id: 2, a: 1, b: 3, as: [{_id: 1, c: 1}]}], leftColl .aggregate([ {$match: {a: 1, b: 3}}, {$lookup: {from: rightColl.getName(), as: "as", localField: "a", foreignField: "c"}} ]) .toArray()); } runQuery(); runQuery(); // We expect the left-hand side to have an active plan cache entry, and expect no plan cache entry // on the right hand side. const leftHandCacheEntries = leftColl.aggregate([{$planCacheStats: {}}]).toArray(); assert.eq(leftHandCacheEntries.length, 1); assert.eq(leftHandCacheEntries[0].isActive, true); const rightHandCacheEntries = rightColl.aggregate([{$planCacheStats: {}}]).toArray(); assert.eq(rightHandCacheEntries.length, 0); // After dropping the index on the right-hand side, we end up triggering a replan repeatedly. assert.commandWorked(rightColl.dropIndex({c: 1})); runQuery(); runQuery(); runQuery(); }());
The test runs a $lookup. On the left-hand side, the multi-planner should choose between an access plan using index {a: 1} and an access plan using index {b: 1}. The {b: 1} index scan is more selective, so we end up inserting an entry into the classic plan cache for {b: 1}. (At the moment, we do not support using the SBE plan cache when $lookup is pushed down into SBE.) No cache entry is created on the right-hand side, which is expected. Currently, the plan is always chosen heuristically on the right-hand side, so multi-planning and plan caching only apply on the left-hand side. The "works" value associated with the cache entry is some small value such as 3, since it takes just a few storage reads to obtain the index key and then fetch the associated document.
The heuristic planning code for the $lookup chooses an indexed nested loop join due to the availability of index {c: 1} on the inner side.
Next, the test drops the {c: 1} and re-runs the query. The access path using index {b: 1} on the left-hand side is recovered from the classic plan cache. The heuristic planning code for the pushed down $lookup runs, but this time must use a nested loop join rather than an index join because index {c: 1} has been dropped. This execution plan requires a full scan of the inner collection. Therefore, during the trial period, we blow past the works budget and a replan is triggered.
Here's where the problem begins. Replanning, like multi-planning in the first place, only considers the access path for the outer side. The portion of the plan implementing $lookup is entirely ignored. So replanning selects {b: 1} once again, and re-inserts a small works value of ~3. This means that we are right back where we began. If you run the query again, it will replan in exactly the same way each time. The plan cache entry is effectively useless for this query.
The way that we are deciding whether or not to replan for $lookup seems wrong. Since the plan cache entry only tells you something about the expected performance of the access plan on the left-hand side, we shouldn't count reads done against the right-hand side collection when running the cached plan trial period. Exactly how to implement this is not obvious, since we definitely want to be running the complete SBE plan during the trial period.
- related to
-
SERVER-65199 replanReason is incorrectly omitted from logs when replanning a query with $group/$lookup pushed down to SBE
- Closed