Skip to content

feat(NODE-7452): restrict server deprioritization on replica sets to overload errors#4875

Open
tadjik1 wants to merge 5 commits intomainfrom
NODE-7452
Open

feat(NODE-7452): restrict server deprioritization on replica sets to overload errors#4875
tadjik1 wants to merge 5 commits intomainfrom
NODE-7452

Conversation

@tadjik1
Copy link
Member

@tadjik1 tadjik1 commented Feb 20, 2026

Description

Summary of Changes

  • Restrict server deprioritization during retryable operations so that on replica sets, only SystemOverloadedError triggers 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.
  • New prose tests are implemented.

Second implementation of DRIVERS-3404.

Notes for Reviewers

What is the motivation for this change?

Release Highlight

Release notes highlight

Double check the following

  • Lint is passing (npm run check:lint)
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 SystemOverloadedError retry 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', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 dariakp added the wip label Feb 20, 2026
@dariakp dariakp changed the title feat(NODE-7452): restrict server deprioritization on replica sets to overload errors [DO NOT MERGE] feat(NODE-7452): restrict server deprioritization on replica sets to overload errors Feb 20, 2026
@dariakp
Copy link
Contributor

dariakp commented Feb 20, 2026

Setting a "do not merge" until the spec is merged.

@nbbeeken nbbeeken added Blocked Blocked on other work and removed wip labels Feb 23, 2026
@tadjik1 tadjik1 changed the title [DO NOT MERGE] feat(NODE-7452): restrict server deprioritization on replica sets to overload errors feat(NODE-7452): restrict server deprioritization on replica sets to overload errors Feb 24, 2026
@tadjik1 tadjik1 removed the Blocked Blocked on other work label Feb 24, 2026
@tadjik1 tadjik1 requested review from a team and dariakp February 24, 2026 12:33
@codeowners-service-app
Copy link

Assigned durran for team dbx-node-devs because dariakp is out of office.

nbbeeken
nbbeeken previously approved these changes Feb 25, 2026
@nbbeeken nbbeeken dismissed dariakp’s stale review February 25, 2026 14:43

Daria's request for changes was to prevent accidentally merging this before the spec landed. The spec has landed and the tests have been updated with the steps from the prose.

@tadjik1
Copy link
Member Author

tadjik1 commented Feb 26, 2026

The base was different and therefore I had to fix merge conflicts with current main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants