[SERVER-30263] Perform MinKey/MaxKey checking inside Chunk::shouldSplit, instead of taking minIsInf/maxIsInf parameters Created: 21/Jul/17 Updated: 29/May/18 Resolved: 29/May/18 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Sharding |
| Affects Version/s: | 3.5.10 |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Hugh Han | Assignee: | Matthew Saltz (Inactive) |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Sprint: | Sharding 2018-06-04 |
| Participants: |
| Description |
|
Currently, checking whether a chunk contains the MinKey or MaxKey occurs outside of the Chunk::shouldSplit function, inside cluster_write.cpp. Instead, this check should be contained inside Chunk::shouldSplit itself for better design, and so that we don't have to pass in extra parameters. |
| Comments |
| Comment by Matthew Saltz (Inactive) [ 23/May/18 ] |
|
After looking at this usage, I disagree that it's worth doing as stated. For one, the minIsInf/maxIsInf flags are used inside cluster_write, even after the call to shouldSplit, so we'd either need to compute these flags twice or return them from shouldSplit. Furthermore, shouldSplit actually doesn't even care which of minIsInf or maxIsInf is true, only whether at least one of them is true. If anything, we could change the signature of shouldSplit to take a single bool 'isExtremeChunk' or something like that, though I don't know if that's worth doing, especially since all of this behavior relates to/is required by the top chunk optimization, which we may remove or re-implement on the shard. In either case, I'm not sure that this small code change will still be relevant. |