[SERVER-48618] Remove TODO's for SERVER-48618 Created: 05/Jun/20 Updated: 29/Oct/23 Resolved: 18/Sep/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 4.8.0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Haley Connelly | Assignee: | Haley Connelly |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | sharding-wfbf-day | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||
| Operating System: | ALL | ||||||||||||
| Sprint: | Sharding 2020-07-27, Sharding 2020-08-10, Sharding 2020-08-24, Sharding 2020-09-21, Sharding 2020-10-05 | ||||||||||||
| Participants: | |||||||||||||
| Description |
|
new description We've decided not to do what this ticket originally aimed to do. Remove TODO's for this ticket in the codebase. old description service_entry_point_common execCommandDatabase calls oss.initializeClientRoutingVersionsFromCommand(invocation->ns(), request.body). However, not all commands that go through this path are derived from InvocationBase - thus invocation->ns() yields an empty NamespaceString. List of commands we found that pass an empty NamespaceString to OperationShardingState::initializeClientRoutingVersionsFromCommand() that aren't derived from InvocationBase: Additionally, DropConnectionsCmd derives from InvocationBase but passes an empty NamespaceString to OperationShardingState::initializeClientRoutingVersionsFromCommand().
|
| Comments |
| Comment by Githook User [ 18/Sep/20 ] |
|
Author: {'name': 'Haley Connelly', 'email': 'haley.connelly@mongodb.com', 'username': 'haleyConnelly'}Message: |
| Comment by Haley Connelly [ 17/Sep/20 ] |
|
After some discussion, we've decided not to move forward with the ticket, and are just going to remove TODO's for it instead. |
| Comment by Haley Connelly [ 08/Sep/20 ] |
|
kaloian.manassiev, we discovered a scenario where the OSS tracks an incomplete/ inconsistent namespace, but don't have a good solution for its fix Initializing OSS with incomplete namespace * mongos specifies the UUID rather than the collection name, and explicitly specifies shard version UNSHARDED, to the find command when fetching the post image for unsharded collections (eg: in change streams).
Why the behavior isn't broken today despite OSS inconsistency |
| Comment by Haley Connelly [ 25/Aug/20 ] |
|
Yes, it is possible that the user can provide an empty namespace (it would fail eventually, but with the invariant in place, invariant first). Max brought up a good suggestion offline - if both conditions are true, throw (not invariant) That way, commands that don't need to be versioned won't fail when they should pass. (See DropConnectionsCmd for an example of a command that will always return an empty NamespaceString but shouldn't need to be versioned). |
| Comment by Kaloian Manassiev [ 25/Aug/20 ] |
|
I don't quite understand the question. Are you saying that the namespace passed to the OSS is sometimes derived from how user formulated the command? Because if that is the case, yes we should uassert. If it however is derived from some internal code, then we have a bug and should fix it (therefore adding the invariant). |
| Comment by Haley Connelly [ 20/Aug/20 ] |
|
Its not safe to invariant that the NSS is not empty inside the OperationShardingState. Despite not inheriting from InvocationBase, the listed commands still succeed and provide a valid namespace to the OSS when they aren't specifically designed to fail. kaloian.manassiev We could uassert when the NamespaceString is empty rather than invariant. Or we could leave as is. Which do you prefer? |
| Comment by Haley Connelly [ 05/Jun/20 ] |
|
Throughout OperationShardingState, we should probably enforce that the namespace passed in should not be empty (eg: for getShardVersion etc. as well). |