[SERVER-47261] AsyncRequestSender should populate clientOperationKey Created: 01/Apr/20 Updated: 06/Dec/22 |
|
| Status: | Open |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Benjamin Caimano (Inactive) | Assignee: | Backlog - Service Architecture |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | servicearch-wfbf-day | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||
| Assigned Teams: |
Service Arch
|
||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||
| Sprint: | Service arch 2020-04-20, Service arch 2020-05-04, Service arch 2020-05-18 | ||||||||||||||||
| Participants: | |||||||||||||||||
| Description |
|
When we create a RemoteCommandRequestOnAny in the ARS here, we might already have a client operation key in the command BSON. I think the way to fix this is to move the clientOperationKey logic out of the RemoteCommandRequest here and instead use an ARS-owned clientOperationKey that is added to the metadataObj ahead of time. |
| Comments |
| Comment by Max Hirschhorn [ 08/Apr/22 ] |
|
While not being that familar with clientOperationKey and hedged reads, I do want to call out some consideration may need to be given to the current proposal of using the same clientOperationKey as the originating request around sharded $lookup. It is possible to have the following sequence of network messages exchanged as a result of a single client request:
(The nested sharded $lookup isn't the only way this could happen but is more practical then a $lookup within the same shard not picking the current node as the lowest ping time and it winning the runoff.) |
| Comment by Lauren Lewis (Inactive) [ 07/Apr/22 ] |
|
garaudy.etienne I'd like to hear from anyone who has continued Ben & Spencer's work or who has a vested interest in this issue being worked on. |
| Comment by Lauren Lewis (Inactive) [ 24/Feb/22 ] |
|
We haven’t heard back from you for at least one calendar year, so this issue is being closed. If this is still an issue for you, please provide additional information and we will reopen the ticket. |
| Comment by Benjamin Caimano (Inactive) [ 16/Apr/20 ] |
|
I mentioned it in passing before but, without this change, |
| Comment by Spencer Brody (Inactive) [ 15/Apr/20 ] |
|
I don't really feel like now is a good time to make this change. I don't think this should be done until we're ready to start sending clientOperationKeys on requests outside of hedged reads. Right now the logic for setting clientOperationKey is very tied into logic around using hedged reads. The problem is that we make the decision on whether or not to add clientOperationKey based on both the readPreference for the operation as well as the name of the command being run. While readPreference is shared across all requests in a given ARS, the command being run is not. This makes it difficult to move the logic for setting clientOperationKey up to the ARS since there really is no way to make the decision on whether or not to set clientOperationKey for the entire ARS, the decision is still fundamentally tied to each individual request, so making the decision at the Request layer makes sense. If/when we decide to use clientOperationKey in more places, we will need to make a cohesive plan around what the right layer is to be controlling this and whether or not it is allowed for different requests within the same ARS to have different clientOperationKeys. This seems like a change we should make when we have a use case in mind for it to inform our design decisions, rather than doing it now "just in case" we need it down the road, when we don't have a real idea of the requirements or desired behavior. FYI ben.caimano |