Conversation
Member
Author
|
CI is red only for this change: #4916 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR finalizes the phase 1 client backpressure implementation per updated spec (DRIVERS-3427 / NODE-7491) by removing the deferred token-bucket mechanism, introducing new client/URI options to control overload retry behavior, and syncing the driver’s retry logic and spec/prose tests to the new max-attempt semantics.
Changes:
- Removed the token bucket implementation (
TokenBucket,adaptiveRetries) and replaced overload retry limits withmaxAdaptiveRetriesplusenableOverloadRetargeting. - Simplified overload retry behavior in the operation execution retry loop and updated transaction commit retry limits to align with
maxAdaptiveRetries. - Updated/synced unified spec tests, URI option tests, and prose/integration tests to reflect the new retry counts and option names; removed obsolete SDAM unified test files.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/connection_string.test.ts | Removes unit assertions for deprecated adaptiveRetries. |
| test/tools/uri_spec_runner.ts | Adds maxAdaptiveRetries and enableOverloadRetargeting to URI option validation mapping. |
| test/spec/uri-options/client-backpressure-options.yml | Updates URI option spec tests from adaptiveRetries to new options. |
| test/spec/uri-options/client-backpressure-options.json | JSON form of updated URI option spec tests. |
| test/spec/transactions/unified/backpressure-retryable-writes.yml | Updates expected retry maxAttempts from 5 to 2 for backpressure-labeled writes. |
| test/spec/transactions/unified/backpressure-retryable-writes.json | JSON form of updated retryable writes backpressure tests. |
| test/spec/transactions/unified/backpressure-retryable-reads.yml | Updates expected retry maxAttempts from 5 to 2 for backpressure-labeled reads. |
| test/spec/transactions/unified/backpressure-retryable-reads.json | JSON form of updated retryable reads backpressure tests. |
| test/spec/transactions/unified/backpressure-retryable-commit.yml | Updates expected retry maxAttempts from 5 to 2 for commitTransaction. |
| test/spec/transactions/unified/backpressure-retryable-commit.json | JSON form of updated commitTransaction backpressure tests. |
| test/spec/transactions/unified/backpressure-retryable-abort.yml | Updates expected retry maxAttempts from 5 to 2 for abortTransaction. |
| test/spec/transactions/unified/backpressure-retryable-abort.json | JSON form of updated abortTransaction backpressure tests. |
| test/spec/server-discovery-and-monitoring/unified/backpressure-network-timeout-fail.yml | Removes obsolete unified SDAM backpressure test. |
| test/spec/server-discovery-and-monitoring/unified/backpressure-network-timeout-fail.json | JSON form removal of obsolete unified SDAM backpressure test. |
| test/spec/server-discovery-and-monitoring/unified/backpressure-network-error-fail.yml | Removes obsolete unified SDAM backpressure test. |
| test/spec/server-discovery-and-monitoring/unified/backpressure-network-error-fail.json | JSON form removal of obsolete unified SDAM backpressure test. |
| test/spec/client-backpressure/README.md | Updates prose spec guidance: retry counts, prerequisites, removes token-bucket test. |
| test/spec/client-backpressure/getMore-retried.yml | Updates getMore backpressure retry counts to maxAttempts=2. |
| test/spec/client-backpressure/getMore-retried.json | JSON form of updated getMore backpressure tests. |
| test/spec/client-backpressure/backpressure-retry-max-attempts.yml | Updates generated unified suite expectations to maxAttempts=2. |
| test/spec/client-backpressure/backpressure-retry-max-attempts.json | JSON form of updated “retry max attempts” suite. |
| test/spec/client-backpressure/backpressure-retry-loop.yml | Updates retry loop suite to match maxAttempts=2 and related event expectations. |
| test/spec/client-backpressure/backpressure-retry-loop.json | JSON form of updated retry loop suite. |
| test/spec/client-backpressure/backpressure-connection-checkin.yml | Adjusts expected CMAP events to reflect fewer retries. |
| test/spec/client-backpressure/backpressure-connection-checkin.json | JSON form of updated connection check-in expectations. |
| test/mongodb_bundled.ts | Stops exporting removed token-bucket constants from test bundle. |
| test/mongodb_all.ts | Stops exporting removed src/token_bucket module from test bundle. |
| test/integration/retryable-writes/retryable_writes.spec.prose.test.ts | Updates expected attempt counts to reflect default maxAdaptiveRetries=2. |
| test/integration/retryable-reads/retryable_reads.spec.prose.test.ts | Gates overload retargeting prose tests behind enableOverloadRetargeting and adds disabled-case coverage. |
| test/integration/client-backpressure/client-backpressure.spec.test.ts | Updates skipped-test key string to match new maxAttempts=2 wording. |
| test/integration/client-backpressure/client-backpressure.prose.test.ts | Updates prose backpressure tests: removes token bucket checks; aligns retry/backoff expectations with maxAdaptiveRetries defaults/config. |
| src/token_bucket.ts | Removes token-bucket implementation and related constants. |
| src/sessions.ts | Uses maxAdaptiveRetries to set commitTransaction retry limits. |
| src/sdam/topology.ts | Removes token bucket state; adds new topology options maxAdaptiveRetries and enableOverloadRetargeting. |
| src/operations/execute_operation.ts | Simplifies overload retry loop: retries up to maxAdaptiveRetries, retains exponential backoff, and gates non-sharded retargeting by enableOverloadRetargeting. |
| src/mongo_client.ts | Adds new public client options and makes them part of the resolved required options set. |
| src/index.ts | Removes exports related to deleted token-bucket types/constants. |
| src/connection_string.ts | Adds URI parsing support and defaults for maxAdaptiveRetries and enableOverloadRetargeting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
67
to
72
| - client: *client | ||
| events: | ||
| # we expect 6 pairs of command started and succeeded events: | ||
| # 1 initial attempt and 5 retries. | ||
| - commandStartedEvent: | ||
| commandName: listDatabases | ||
| - commandFailedEvent: | ||
| commandName: listDatabases | ||
| - commandStartedEvent: | ||
| commandName: listDatabases | ||
| - commandFailedEvent: | ||
| commandName: listDatabases | ||
| - commandStartedEvent: | ||
| commandName: listDatabases | ||
| - commandFailedEvent: | ||
| commandName: listDatabases | ||
| # we expect 3 pairs of command started and succeeded events: | ||
| # 1 initial attempt and 2 retries. | ||
| - commandStartedEvent: | ||
| commandName: listDatabases |
Member
Author
There was a problem hiding this comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Summary of Changes
Finalizes the client backpressure implementation for phase 1 rollout per the updated specification (DRIVERS-3427).
TokenBucketclass,adaptiveRetriesoption) — deferred to a future phasemaxAdaptiveRetriesclient/URI option (non-negative integer, default 2) — controls max retry attempts for overload errorsenableOverloadRetargetingclient/URI option (boolean, default false) — gates server deprioritization on overload errorsexecute_operation.ts: retries now always proceed up tomaxAdaptiveRetrieswithout token budget checkssessions.tswithTransactioncommit logic to usemaxAdaptiveRetriesNotes for Reviewers
The sharded topology deprioritization (retrying on a different mongos) is not gated behind
enableOverloadRetargeting— per spec, that behavior is independent and always active for sharded clusters. Only theSystemOverloadedErrordeprioritization path across non-sharded topologies is controlled by the new flag.What is the motivation for this change?
NODE-7491
Release Highlight
Added support for MongoDB's Intelligent Workload Management
Added support for MongoDB's Intelligent Workload Management (IWM) and ingress connection rate limiting features. The driver now gracefully handles write-blocking scenarios and optimizes connection establishment during high-load conditions to maintain application availability.
Two new client options are available:
maxAdaptiveRetries(default: 2) - configures the maximum number of retries when the server returns an overload error. Set to 0 to disable overload retries.enableOverloadRetargeting(default: false) - when enabled, the driver will deprioritize servers that return overload errors during retry server selection.Double check the following
npm run check:lint)type(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript