[SERVER-34644] Disable `DollarPrefixedFieldName` checks for moveChunk/mergeChunks Created: 24/Apr/18 Updated: 29/Oct/23 Resolved: 30/Apr/18 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Sharding |
| Affects Version/s: | None |
| Fix Version/s: | 3.4.16, 3.6.5, 4.0.0-rc0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Kaloian Manassiev | Assignee: | Kaloian Manassiev |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||||||
| Backport Requested: |
v3.6, v3.4
|
||||||||||||||||||||||||||||
| Sprint: | Sharding 2018-05-07 | ||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||
| Case: | (copied to CRM) | ||||||||||||||||||||||||||||
| Description |
|
The moveChunk/mergeChunks commands do not introduce any new chunk boundaries that do not already exist. Disallowing specifying shard keys with $ in the field names that they accept prevents fixing sharding routing information, which might inadvertently have been created by older versions of the server. Because of this we should disable these checks. |
| Comments |
| Comment by Githook User [ 30/May/18 ] |
|
Author: {'username': 'kaloianm', 'name': 'Kaloian Manassiev', 'email': 'kaloian.manassiev@mongodb.com'}Message: (cherry picked from commit cc7a103bd71ee98ad85e1b5dcca8763b4eb16679) |
| Comment by Githook User [ 01/May/18 ] |
|
Author: {'email': 'kaloian.manassiev@mongodb.com', 'name': 'Kaloian Manassiev', 'username': 'kaloianm'}Message: |
| Comment by Githook User [ 01/May/18 ] |
|
Author: {'email': 'kaloian.manassiev@mongodb.com', 'name': 'Kaloian Manassiev', 'username': 'kaloianm'}Message: |
| Comment by Githook User [ 01/May/18 ] |
|
Author: {'email': 'kaloian.manassiev@mongodb.com', 'name': 'Kaloian Manassiev', 'username': 'kaloianm'}Message: (cherry picked from commit cc7a103bd71ee98ad85e1b5dcca8763b4eb16679) |
| Comment by Githook User [ 30/Apr/18 ] |
|
Author: {'email': 'kaloian.manassiev@mongodb.com', 'username': 'kaloianm', 'name': 'Kaloian Manassiev'}Message: |
| Comment by Githook User [ 30/Apr/18 ] |
|
Author: {'email': 'kaloian.manassiev@mongodb.com', 'username': 'kaloianm', 'name': 'Kaloian Manassiev'}Message: |
| Comment by Githook User [ 26/Apr/18 ] |
|
Author: {'email': 'kaloian.manassiev@mongodb.com', 'username': 'kaloianm', 'name': 'Kaloian Manassiev'}Message: There is never a reason to construct ShardKeyPattern with an invalid |
| Comment by David Storch [ 26/Apr/18 ] |
|
This all sounds reasonable to me. I didn't spot any issues based on the description of your planned work above.
That's my reading of the code as well. The "slow path" is to invoke the query planner and then analyze the bounds over a hypothetical index whose key pattern is the shard key pattern. If the routing table indicates that the set of chunks which intersect with the bounds all reside on one shard, then only that shard will be targeted. |
| Comment by Kaloian Manassiev [ 25/Apr/18 ] |
|
It means that we will skip the fast path and will drop to getIndexBoundsForQuery, which presumably will see that the query does not contain an $-operator such as $gle, etc and probably will still target a single shard? david.storch? |
| Comment by Andy Schwerin [ 25/Apr/18 ] |
|
I think this is OK. The current query targeting behavior means that you cannot target a write to a single shard using the shard key if the shard key value contains a $ sign? |
| Comment by Kaloian Manassiev [ 25/Apr/18 ] |
|
The code which checks for the presence of $-prefixed key names is buried at a very low level in the sharding code and is invoked for all operations which have anything to do with shard keys. In addition, from what I understand, it has some side-effects which do not always result in error, but instead treat the BSON as not containing a shard key so that the more specialized query targeting code can run and take into account any operators. What I propose to do is the following:
david.storch, schwerin - what do you think about this plan? I would like to do it in master first and then it will be backported to 3.6. I think upgrade to 3.4 might not be necessary, because 3.4.14 works alright with "illegal" chunks and does not print any warnings, it's just that they cannot be merged in 3.4. |