[SERVER-33710] Support readConcern snapshot with atClusterTime in cluster distinct command Created: 06/Mar/18  Updated: 29/Oct/23  Resolved: 08/Aug/18

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

Type: Task Priority: Major - P3
Reporter: Misha Tyulenev Assignee: Cheahuychou Mao
Resolution: Fixed Votes: 0
Labels: ShardedTxn:GlobalSnapshot
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Documented
is documented by DOCS-11964 Docs for SERVER-33710: Support readCo... Closed
Backwards Compatibility: Fully Compatible
Sprint: Sharding 2018-08-13
Participants:

 Description   

As part of this ticket:

  • Ensure ClusterDistinct supports readConcern level snapshot
  • Ensure ClusterDistinct selects an appropriate atClusterTime based on the shards it targets, and will retry on snapshot errors
  • Add unit tests verifying this works

Proposed implementation:

  1. Add a supportsReadConcern to ClusterDistinct that always returns true
  2. Update functions in cluster_commands_helpers.cpp to compute atClusterTime and attach it to the generated responses, like in ClusterAggregate (computing atClusterTime with computeAtClusterTime, attaching atClusterTime with appendAtClusterTime)
    • buildVersionedRequestsForTargetedShards here
    • buildUnversionedRequestsForShards here
  3. snapshot errors are already retried in strategy.cpp, so there should be no work there
  4. Add unit test verify requests sent by ClusterDistinct contain appropriate atClusterTimes and are retried on snapshot errors, like the existing ClusterFind tests


 Comments   
Comment by Githook User [ 08/Aug/18 ]

Author:

{'username': 'cheahuychou', 'name': 'Cheahuychou Mao', 'email': 'cheahuychou.mao@mongodb.com'}

Message: SERVER-33710 Support readConcern snapshot with atClusterTime in cluster distinct command
Branch: master
https://github.com/mongodb/mongo/commit/957f2abc510016546a577e639b2bfabb182c4e17

Comment by Misha Tyulenev [ 01/Aug/18 ]

lgtm

Comment by Esha Maharishi (Inactive) [ 01/Aug/18 ]

Thanks, lgtm.

Comment by Jack Mulrow [ 01/Aug/18 ]

It seems like mongos's distinct already supports readConcern (as it should, since it's a read command). Is there something more to do?

No, we shouldn't have to do anything then.

It would be nice to avoid calling these in each command path. Is there a way we can place these calls above (in the service entry point) or below (in the ShardingTaskExecutor) the Command's run()? Will we try to reorganize the code to be able to do that under SERVER-36237, and if so, should we block this ticket on SERVER-36237?

Like we talked about offline, SERVER-36237 will include refactoring how we attach atClusterTime to outgoing requests, but since we'll still have to compute atClusterTime in the targeting layer and need test coverage for distinct, the changes here will just be a small addition to the refactor, so there's no need to block this on SERVER-36237.

Comment by Esha Maharishi (Inactive) [ 01/Aug/18 ]

Looks good, two questions:

1. Add a supportsReadConcern to ClusterDistinct that always returns true

It seems like mongos's distinct already supports readConcern (as it should, since it's a read command). Is there something more to do?

2. Update functions in cluster_commands_helpers.cpp to compute atClusterTime and attach it to the generated responses, like in ClusterAggregate (computing atClusterTime with computeAtClusterTime, attaching atClusterTime with appendAtClusterTime)

It would be nice to avoid calling these in each command path. Is there a way we can place these calls above (in the service entry point) or below (in the ShardingTaskExecutor) the Command's run()? Will we try to reorganize the code to be able to do that under SERVER-36237, and if so, should we block this ticket on SERVER-36237?

Comment by Jack Mulrow [ 01/Aug/18 ]

misha.tyulenev esha.maharishi, can you guys review the proposed implementation above?

Generated at Thu Feb 08 04:34:19 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.