[SERVER-51731] ChunkManager::getShardIdsForRange should not hardcode isMaxInclusive to true Created: 19/Oct/20  Updated: 23/Oct/20  Resolved: 23/Oct/20

Status: Closed
Project: Core Server
Component/s: Sharding
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Cheahuychou Mao Assignee: Daniel Gottlieb (Inactive)
Resolution: Duplicate Votes: 0
Labels: sharding-wfbf-day
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
duplicates SERVER-20768 Mongos find query including upper bou... Backlog
Operating System: ALL
Participants:

 Description   

getShardIdsForRange currently hardcodes isMaxInclusive to true and that causes mongos to target both of the shards in the following case.

const st = ShardingTest({shards: 2});
 
assert.commandWorked(st.s.adminCommand({enableSharding: "foo"}));
st.ensurePrimaryShard("foo", st.shard0.shardName);
 
assert.commandWorked(st.s.adminCommand({shardCollection: "foo.bar", key: {_id: 1}}));
assert.commandWorked(st.s.adminCommand({split: "foo.bar", middle: {_id: 10}}));
assert.commandWorked(st.s.adminCommand({moveChunk: "foo.bar", find: {_id: 10}, to: st.shard1.shardName, _waitForDelete: true}));
 
assert.commandWorked(st.s.getDB("foo").runCommand({find: "bar", filter: {_id: {$lt: 10}}}));
st.stop();



 Comments   
Comment by Daniel Gottlieb (Inactive) [ 23/Oct/20 ]

This turns out to be a dup of SERVER-20768.

Because I've chased this down, I'll write up my observations (even though SERVER-20768 and the trickle of other dupes linked to that over the years basically includes all of them):

  • The claim is correct, doing a range query on the exclusive upper bound of a chunk will needlessly scatter the request to an additional shard (assuming the shard for the following chunk is not already required for some other chunk covered by the range).
  • I don't see any reason to believe this is a correctness problem.
  • From what I can tell, the code has been this way since at least 2012.
  • When generating IndexBounds, the appropriate inclusive/exclusive properties are preserved.
  • When flattening the IndexBounds into a BoundList, the granularity is lost. A BoundsList is documented as inclusive on both ends.
  • Unconditionally changing isMaxInclusive from true to false would be a correctness bug (this was never suggested, but figured I'd put it out there).

As far as making a change goes, I have can think of an obvious adjustment to the algorithm for preserving any exclusive upper bounds when computing the BoundList, but it's not obvious to me it's correct. I don't have any reason to believe it's wrong, but this isn't something I'd feel confident guessing at.

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