DRIVERS-3326: clarify retry behavior when errors with NoWritesPerformed are encountered #1878
+10
−6
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR clarifies the behavior when multiple errors with NoWritesPerformed are encountered (potentially possible when CSOT is enabled, or in the future with backpressure).
I wanted to add testing, but I ran into some difficulties that led me to believe it probably isn't worth adding test infrastructure just for this test.
To get around this, I wrote a simple JS proxy that added an incrementing counter to the payload of each response, so that the error a user receives has an identifier. This worked, but when I went to test this, I ran into another issue: to test this scenario, we need multiple retryable writes (CSOT or backpressure). Backpressure hasn't merged yet, so we can't rely on that feature. When I tried using CSOT, I could not actually reproduce this scenario because every CSOT error manifested as a timeoutMS error (which does not have a NoWritesPerformed error label).
One option is to make this PR depend on the client backpressure work and to put my proxy into drivers-evergreen-tools. This still might be more trouble than it's worth, because this is an additional CI variant for drivers, just for one test. The proxy must:
Ultimately, I decided the testing here is more trouble than it's worth. Happy to reconsider if reviewers feel differently though.
Please complete the following before merging:
clusters).