[SERVER-45686] Increase topologyVersion and respond to waiting isMasters on mock State Change Errors from the failCommand failpoint Created: 21/Jan/20  Updated: 27/Oct/23  Resolved: 31/Jan/20

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

Type: Task Priority: Major - P3
Reporter: Shane Harvey Assignee: Jason Chan
Resolution: Gone away Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Sprint: Repl 2020-02-10
Participants:

 Description   

When the server responds with a State Change Errors from the failCommand failpoint, it should also increase topologyVersion and respond to waiting isMasters. The Drivers team uses failCommand extensively in spec tests for retryable writes+reads. Without this change, it takes the client ~10 seconds (maxAwaitTimeMS) to rediscover the server's state.

For example:

  1. client configures a failCommand with NotMaster
  2. client runs a retryable write against Primary P
  3. client observes a NotMaster error and sets P to Unknown
  4. client runs the retry attempt which blocks until P is rediscovered
  5. P's Monitor is blocked for 10 seconds waiting for an awaitable isMaster response

After this change to 10 seconds hang should be removed:

  1. client configures a failCommand with NotMaster
  2. client runs a retryable write against Primary P
  3. client observes a NotMaster error and sets P to Unknown
  4. client runs the retry attempt which blocks until P is rediscovered
  5. P's Monitor immediately receives an awaitable isMaster response and set P to Primary
  6. client retry attempt succeeds ASAP


 Comments   
Comment by Tess Avitabile (Inactive) [ 30/Jan/20 ]

Ah, thank you for figuring that out! Should this ticket be closed?

jason.chan, I'm sorry we didn't catch this before you wrote the code.

Comment by Shane Harvey [ 30/Jan/20 ]

After testing against the latest server (v4.3.3-54-gd1fe174) I no longer believe failCommand with a State Change Error is a problem. Sorry for the confusion. At the time I was testing with a server that did not add topologyVersion to State Change Errors. Now that I'm testing with a server that does, this is the behavior I see: 

  1. client configures a failCommand with NotMaster
  2. client runs a retryable write against Primary P, with topologyVersion.counter = 1
  3. client observes a NotMaster error with topologyVersion.counter = 1  and does not set the server to unknown because the error's topologyVersion is less-or-equal to the current server's
  4. client runs the retry attempt which selects P and succeeds immediately

We still have to deal with a similar problem (10-second pauses) with failCommand that closes the connection but I think we will solve it on the client side.

Hopefully, in practice the retry will not take 10 seconds. The driver should have an up-to-date status for the primary, so it will retry against the primary. Does that sound right to you? Maybe I'm missing something.

Please ignore this. The current behavior of topologyVersion solves this problem too.

Comment by Tess Avitabile (Inactive) [ 30/Jan/20 ]

jason.chan and I discussed this.

  1. I want to push back on incrementing topologyVersion for mongos/standalone. Right now we have assumptions in the server that the topologyVersion never changes on mongos/standalone, and I prefer not to just that assumption for tests. However, I understand the pain of having tests take longer. Is there another solution that might work here? For example, the test could use a lower maxAwaitTimeMS against mongos/standalone. Or we could add a failpoint to make isMaster responses return immediately.
  2. On a real shutdown, the isMaster would either return a ShutdownError or the connection would close, but the topologyVersion does not get bumped. When the server restarts, the topologyVersion has a new processId and a counter of 0. I'm okay with bumping the topologyVersion in the failCommand failpoint, even though it differs from the behavior in a real shutdown.

When directly connected to a secondary, every write will fail with NotMaster and trigger a retry. If the server does not increment the topologyVersion in practice then each retry will take 10 seconds. This seems like an oversight/bug in retryable reads/writes that can be addressed separately.

Hopefully, in practice the retry will not take 10 seconds. The driver should have an up-to-date status for the primary, so it will retry against the primary. Does that sound right to you? Maybe I'm missing something.

Comment by Shane Harvey [ 29/Jan/20 ]

1. Yes, topologyVersion needs to change on mongos anytime it returns a State Change Error to the client. Otherwise, a NotMaster error returned from a mongos could cause a driver's retryable write/read to take 10 seconds. Drivers reset a server to Unknown on State Change Errors regardless if it's a standalone, mongos, or replica set member so I think standalones should also have the same behavior. 
2. I think the answer is also yes. Can you explain the expected behavior on a real shutdown?  

Number 1 brings up an interesting case. When directly connected to a secondary, every write will fail with NotMaster and trigger a retry. If the server does not increment the topologyVersion in practice then each retry will take 10 seconds. This seems like an oversight/bug in retryable reads/writes that can be addressed separately. 

Comment by Jason Chan [ 29/Jan/20 ]

shane.harvey, some requirements questions came up during the implementation we are hoping you could help answer:
1. Does Drivers expect the topologyVersion to be incremented even for mongos/standalones? We expect the answer to be 'no' since we don't expect them to ever increment their TopologyVersion in practice.

2. The design includes the category of ShutdownErrors as a State Change Error. Does Drivers still expect the TopologyVersion to be incremented on shutdown errors? This does not simulate the true server behaviour since the server won't increment TopologyVersion on a real shutdown, but we are wondering if this would be helpful for drivers spec tests.

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