[DO NOT MERGE] feat(NODE-7452): restrict server deprioritization on replica sets to overload errors#4875
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements restricted server deprioritization for retryable operations on replica sets, ensuring that only SystemOverloadedError triggers deprioritization. For sharded clusters, the existing behavior of always deprioritizing failed servers is maintained. This is the second implementation of DRIVERS-3404.
Changes:
- Modified retry logic to restrict server deprioritization on non-sharded topologies to only
SystemOverloadedError - Added prose tests verifying that retryable reads with
SystemOverloadedErrorretry on different servers - Added prose tests verifying that retryable reads with other retryable errors retry on the same server
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/operations/execute_operation.ts | Added conditional logic to restrict server deprioritization based on topology type and error labels |
| test/integration/retryable-reads/retryable_reads.spec.prose.test.ts | Added new prose tests for replica set retry behavior with and without SystemOverloadedError |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| }); | ||
|
|
||
| describe('Retrying Reads in a Replica Set', () => { |
There was a problem hiding this comment.
Could you copy paste the actual prose test steps on each line as a comment, including any numbering of steps or cases (and make sure the describe and it blocks mirror the prose text as much as possible) - we do this as a matter of best practice when implementing prose tests so that they're easier to audit (and it helps in the rare cases when prose tests in the spec are later amended). You can follow the example of the case above this one.
dariakp
changed the title
|
Setting a "do not merge" until the spec is merged. |
2 participants
Description
Summary of Changes
SystemOverloadedErrortriggers deprioritization. For non-overload transient errors, the retry goes back to the same server since it was selected for a reason and is still the best candidate.Second implementation of DRIVERS-3404.
Notes for Reviewers
What is the motivation for this change?
Release Highlight
Release notes highlight
Double check the following
npm run check:lint)type(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript