[SERVER-32150] Only increment scanAndOrder once per logical operation Created: 01/Dec/17 Updated: 06/Dec/22 |
|
| Status: | Backlog |
| Project: | Core Server |
| Component/s: | Aggregation Framework, Diagnostics |
| Affects Version/s: | 3.4.0, 3.6.0 |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Charlie Swanson | Assignee: | Backlog - Query Execution |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Query Execution
|
| Participants: | |
| Case: | (copied to CRM) |
| Description |
|
We will increment the scanAndOrder metric once per operation, which includes getMore operations. This makes for the following strange behaviors:
Original DescriptionCurrently each operation that reports to have a sort stage will increment the global serverStatus counter "metrics.operation.scanAndOrder". When an aggregation is issued against a sharded cluster, this number can be incremented twice, once for the shards half of the pipeline, and once for the merging half (see also This arguably makes sense, since from the mongod's perspective there are two operations performing a blocking sort. On the other hand there was only one operation that performed a blocking sort from the client's perspective. For one last perspective, we did technically scan and order documents twice for that operation. If this metric is tracking the number of times the server performed a blocking sort, then we should increment this once per blocking $sort stage, not once per operation. |
| Comments |
| Comment by Charlie Swanson [ 15/Dec/17 ] | ||||||||||||||||||||||||||||
|
Oh - yea I found an easier way to test my above hypothesis, if I play with the batch size I can force more getMores to happen. Looks like this is just being incremented on each getMore, but a max of once per operation. Note this is tested on 3.6, which for operations like the following without allowDiskUse will perform the merge on mongos, so there is no merging operation happening on either shard.
Compared to without specifying batch size:
I wouldn't really consider this conclusive evidence, but combined with my above comment and the priority of 'Backlog', I'm going to stop there. We can reconsider when this ticket is scheduled in the future. | ||||||||||||||||||||||||||||
| Comment by Charlie Swanson [ 15/Dec/17 ] | ||||||||||||||||||||||||||||
|
So Akira's reproduction script definitely is accurate, it looks like we're incrementing it twice for the aggregation in that example. I can't figure out why though. It looks like the only place we ever touch the scanAndOrderCounter is in recordCurOpMetrics(), and that will only do it one time if the operation reports to have a sort stage at all, we don't ask how many sorts were performed by the operation (maybe we should?). In any case, it looks like we're somehow calling recordCurOpMetrics() multiple times, which is strange because it looks like the only callers of that are from within write commands and right before we return the response from ServiceEntryPointMongod::handleRequest(). I'm investigating how and why this is called twice, but I'm gonna have to dig a little bit and set up a debug build with a breakpoint. Will post back if I find anything. My initial hypothesis is that it's being incremented once on the cursor establishment, and again on the getMore. | ||||||||||||||||||||||||||||
| Comment by Asya Kamsky [ 04/Dec/17 ] | ||||||||||||||||||||||||||||
|
From user POV, it seems like most important is to not ever increment when sort is satisfied by an index, or is a simple merge. Second in importance is not over-counting actual in-memory sorts that should be counted. | ||||||||||||||||||||||||||||
| Comment by Charlie Swanson [ 01/Dec/17 ] | ||||||||||||||||||||||||||||
|
david.storch I put this in "Needs Triage" so that we can discuss what we believe the correct answer to be. We discussed earlier that the number of times this counter increments is not as important as whether it increments at all, so we could also just move this to the backlog and re-visit later. |