[SERVER-40909] push down $skip stage to query when possible Created: 30/Apr/19 Updated: 29/Oct/23 Resolved: 05/Oct/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Aggregation Framework |
| Affects Version/s: | None |
| Fix Version/s: | 4.9.0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Asya Kamsky | Assignee: | Nikita Lapkov (Inactive) |
| Resolution: | Fixed | Votes: | 1 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||
| Issue Links: |
|
||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||
| Sprint: | Query 2020-09-07, Query 2020-09-21, Query 2020-10-05, Query 2020-10-19 | ||||||||||||||||
| Participants: | |||||||||||||||||
| Case: | (copied to CRM) | ||||||||||||||||
| Linked BF Score: | 0 | ||||||||||||||||
| Description |
|
Not sure why we don't do this:
In the first case we don't push anything down, in the second case we push down limit(3) - sum of skip and limit values - but in both cases we then do the skip in agg in the next stage. |
| Comments |
| Comment by Nikita Lapkov (Inactive) [ 06/Oct/20 ] | ||||||||||||
|
Benchmarks results arrived. I have picked the closest tested commit after this change was merged. The results are:
| ||||||||||||
| Comment by Githook User [ 05/Oct/20 ] | ||||||||||||
|
Author: {'name': 'Nikita Lapkov', 'email': 'nikita.lapkov@mongodb.com', 'username': 'laplab'}Message: | ||||||||||||
| Comment by Nikita Lapkov (Inactive) [ 01/Oct/20 ] | ||||||||||||
|
We need to wait to accumulate enough history for new tests on master branch. The plan is to push changes on Monday. | ||||||||||||
| Comment by Githook User [ 01/Oct/20 ] | ||||||||||||
|
Author: {'name': 'Nikita Lapkov', 'email': 'nikita.lapkov@mongodb.com', 'username': 'laplab'}Message: PERF-1989 Add benchmarks to measure impact of | ||||||||||||
| Comment by Nikita Lapkov (Inactive) [ 22/Sep/20 ] | ||||||||||||
|
Patch is ready to merge, Evergreen is okay. Marking this as blocked until PERF-1989 is resolved. | ||||||||||||
| Comment by Ian Boros [ 15/Sep/20 ] | ||||||||||||
|
We should consider doing PERF-1989 to deal with the $skips that get added to the microbenchmarks before merging this. | ||||||||||||
| Comment by Nikita Lapkov (Inactive) [ 11/Sep/20 ] | ||||||||||||
|
Turns out I was measuring patch with --opt=off. Here are updated results for builds with --opt=on.
Note: 32 threads consistently gave me 0 ops/sec for both builds for some reason, so I have used 8 threads instead. | ||||||||||||
| Comment by Nikita Lapkov (Inactive) [ 10/Sep/20 ] | ||||||||||||
|
After talking with bernard.gorman and david.storch we decided to benchmark PROJECT => SORT => SKIP and SORT => SKIP => PROJECT plans on some case where the latter should perform better. I have used aggregation pipeline $sort => $project => $skip for all tests, where $skip omits all documents from collection except one. The projection and sample document were taken from The same query was run on 2 builds:
First build was producing SORT => SKIP => PROJECT plan for the query, second build returned PROJECT => SORT => SKIP plan. Each build was configured as described here and started like this: numactl --physcpubind=12,13,14,15 -i 0 ./build/install/bin/mongod --replSet rs0 --port 50000 --bind_ip localhost Benchmark query was executed using benchrun.py for 1 thread and for 32 threads: numactl --physcpubind=1,2,3 -i 0 python2 benchrun.py --shell ../mongo/build/install/bin/mongo -t THREAD_COUNT --trialCount 5 --port 50000 --replset rs0 -f testcases/huge_projection_pipeline.js --includeFilter aggregation The results are summarized in the table. Each value is ops/second returned by benchrun.py.
What do you think? | ||||||||||||
| Comment by Nikita Lapkov (Inactive) [ 08/Sep/20 ] | ||||||||||||
|
charlie.swanson PROJECT => SORT => SKIP was always chosen, without any conditions. Thank you for idea with $_internalInhibitOptimization! | ||||||||||||
| Comment by Charlie Swanson [ 08/Sep/20 ] | ||||||||||||
|
nikita.lapkovin your most recent performance test did you always prefer PROJECT => SORT => SKIP, or only when the projection is simple, as implied in your previous comment? As a workaround for the tests, you could add an $_internalInhibitOptimization stage in addition to the final $skip. | ||||||||||||
| Comment by Nikita Lapkov (Inactive) [ 08/Sep/20 ] | ||||||||||||
|
I have ran performance test for patch where PROJECT => SORT => SKIP is always preferred over SORT => SKIP => PROJECT (Evergreen). No regressions were detected. Some improvements were found (one, two, three). As mentioned above, most likely the cause is that pipeline did not get any input. | ||||||||||||
| Comment by Nikita Lapkov (Inactive) [ 03/Sep/20 ] | ||||||||||||
|
I have re-applied the original patch and sent it for performance testing again. Unfortunately, it crashed on one of the workloads. To fix this, I have made a small fix described below. To perform $skip pushdown to the query stage we build structure called LimitThenSkip. It represents operation "limit X documents and skip Y after it". This structure is extracted in method DocumentSourceSort::extractSkipAndLimitForPushdown, src/mongo/db/pipeline/document_source_sort.cpp. On certain queries computed skip value was greater than limit. Basically we were asking to skip more documents than the limit stage returned to us. This cause the invariant in function attemptToGetExecutor, src/mongo/db/pipeline/pipeline_d.cpp to fail. To fix it, I have capped the skip value with limit value. Basically it means that we can skip the desired amount of documents but no more than the limit stage returned to us. Now the patch passes the performance test, but the regression described above is still in place (see Aggregation.SortProjectWithBigDocuments in this run). As david.storch pointed out, it is not obvious if we should always choose PROJECT => SORT => SKIP over SORT => SKIP => PROJECT. Now we always do SORT => SKIP => PROJECT which kills the performance on benchmark because big documents are sent to the SORT. My suggestion is to prefer PROJECT => SORT => SKIP plan when the PROJECT is simple. This will fix the regression and seems to be sane assumption. What are your thoughts about that? BTW, if this patch is to be merged into master, mongo-perf workloads will need to be refreshed. As charlie.swanson said, currently some workloads add $skip to the end of aggregation pipeline to prevent BSON serialisation. With the patch applied this skip is going to be pushed down to the query stage and nothing will be returned to pipeline to process. | ||||||||||||
| Comment by David Storch [ 16/Jan/20 ] | ||||||||||||
|
charlie.swanson, to answer your question more directly, pushdown of the $skip inhibits the optimization from The project-sort swapping optimization from | ||||||||||||
| Comment by Ian Boros [ 13/Jan/20 ] | ||||||||||||
|
I could imagine that for cases where the projection is expensive to compute, pushing it beneath the skip means that it will be applied to documents that ultimately get skipped. We had to think about this for Top-K sort during That said, I could see a case for pushing the projection beneath the skip -> sort -> sortkey gen as long as the projection does not contain computed fields (which could cause the document to get bigger). |