[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:
Related
related to SERVER-65329 Ensure cursors are not leaked when he... Open
related to SERVER-63982 Complete TODO listed in SERVER-47261 Open
is related to SERVER-46255 establishCursors/AsyncRequestsSender ... Closed
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:

  1. Application run aggregation with {$readPreference: "nearest"} on unsharded collection.
  2. Mongos chooses ShardA:node0 as its main target and ShardA:node1 as its hedge target. Let's pretend ShardA:node0 responds back first.
  3. The aggregation does a sharded $lookup which targets ShardB. The pipeline within the sharded $lookup contains another sharded $lookup.
  4. ShardA:node0 chooses ShardB:node0 as its main target and ShardB:node1 as its hedge target. Let's pretend ShardB:node1 responds back first.
  5. The aggregation does a sharded $lookup which targets ShardA.
  6. ShardB:node1 chooses ShardA:node0 as its main target and ShardA:node1 as its hedge target. Let's pretend ShardA:node1 responds back first.
  7. If ShardB:node1 runs _killOperation on ShardA:node0 using the same clientOperationKey from #2 then the entire aggregation is killed.

(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, SERVER-46255 in the presence of hedged reads will incur cursor leakage and wasted work without this. This is not a "just in case" sadly. We also expect the ARS to be used for a superset of the hedged reads whitelist. It's worth making this more correct within a reasonable timeframe.

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

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