CSHARP-6089: Client Backpressure with baseBackoffMS - #2065
Open
papafe wants to merge 8 commits into
Open
Conversation
papafe
changed the title
Update the exponential-backoff formula from 2^(attempt-1) to 2^attempt per the client-backpressure and transactions-convenient-api spec change (mongodb/specifications#1953), and update the dependent backoff tests. Also adds the baseBackoffMS overload integration test.
…and adapt tests to the new OperationContext API - Add Feature.ClientBackpressureBaseBackoffMs (wire version Server90) and gate the integration test on it instead of a server-version string, so it runs on 9.0 pre-release servers rather than skipping until GA. - Adopt the updated spec test 5 shape (absolute backoff bounds + assert the driver parsed baseBackoffMS) for the mocked unit test. - Fix GetOperationRetryBackoffDelay test bounds for the 2^attempt exponent. - Adapt tests to the CSHARP-6011 change (Session moved onto OperationContext; OperationContext(TimeSpan, CancellationToken) and NoTimeout removed).
The async retry path slept the full backpressure backoff via Task.Delay(backoff, operationContext.CancellationToken), which does not fire when the CSOT deadline elapses, so a large server-supplied baseBackoffMS could overshoot the deadline by seconds and surface a late TimeoutException. Add the same early-bail guard the sync path already has (throw the original overload error when the backoff would exceed the remaining timeout), matching the client-backpressure spec's deadline check. Add regression tests to both executor test suites and correct the AGENTS.md note that wrongly claimed the async path needed no guard.
The client-backpressure and handshake spec PR is merged, so reference the merged tests READMEs (client-backpressure Test 5, handshake Test 9) instead of the pull request.
Contributor
Author
|
The spec PR has been merged. I've updated the test comment links and now this is ready for review. |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the CSHARP-6089 client backpressure spec updates across the driver core and spec tests, including the handshake backpressure field versioning, server-provided baseBackoffMS overrides on overload errors, updated exponential backoff math (2^attempt), and CSOT-deadline-aware async backoff behavior.
Changes:
- Update handshake/hello
backpressurevalue fromtrueto string"2"and adjust related tests. - Parse and honor server-supplied
baseBackoffMSfrom overload errors when computing adaptive retry backoff. - Align retry backoff exponent to
2^attemptand ensure the async retry path bails out when backoff would exceed the remaining CSOT deadline.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/Specifications/mongodb-handshake/prose-tests/MongoDbHandshakeProseTests.cs | Update handshake prose test to assert backpressure: "2". |
| tests/MongoDB.Driver.Tests/Specifications/client-backpressure/prose-tests/ClientBackpressureProseTests.cs | Add/adjust prose tests for baseBackoffMS override and new backoff totals. |
| tests/MongoDB.Driver.Tests/Core/WireProtocol/CommandWriteProtocolTests.cs | Assert baseBackoffMS is preserved on command exceptions and readable via helper. |
| tests/MongoDB.Driver.Tests/Core/Servers/ServerMonitorTests.cs | Update expected hello payloads to backpressure : '2'. |
| tests/MongoDB.Driver.Tests/Core/Servers/RoundTripTimeMonitorTests.cs | Update expected hello payloads to backpressure : '2'. |
| tests/MongoDB.Driver.Tests/Core/Operations/RetryableWriteOperationExecutorTests.cs | Add unit coverage for baseBackoffMS backoff base and CSOT-deadline-aware backoff. |
| tests/MongoDB.Driver.Tests/Core/Operations/RetryableReadOperationExecutorTests.cs | Add unit coverage for baseBackoffMS backoff base and CSOT-deadline-aware backoff. |
| tests/MongoDB.Driver.Tests/Core/Operations/RetryabilityHelperTests.cs | Update backoff exponent expectations and add tests for baseBackoffMS parsing and override application. |
| tests/MongoDB.Driver.Tests/Core/Connections/HelloHelperTests.cs | Update hello helper expectations to include backpressure: '2'. |
| tests/MongoDB.Driver.Tests/ClientSessionHandleTests.cs | Adjust transaction retry backoff timing assertion for new exponent behavior. |
| tests/MongoDB.Driver.TestHelpers/Core/CoreExceptionHelper.cs | Add helper overload to build MongoCommandException from a custom result document. |
| src/MongoDB.Driver/Core/Operations/RetryableWriteOperationExecutor.cs | Apply baseBackoffMS override and add CSOT remaining-time guard before async delays. |
| src/MongoDB.Driver/Core/Operations/RetryableReadOperationExecutor.cs | Apply baseBackoffMS override and add CSOT remaining-time guard before async delays. |
| src/MongoDB.Driver/Core/Operations/RetryabilityHelper.cs | Update backoff math to backoffBase^attempt, implement baseBackoffMS parsing, and plumb override into operation backoff. |
| src/MongoDB.Driver/Core/Operations/AGENTS.md | Update operational guidance to reflect async CSOT deadline guarding. |
| src/MongoDB.Driver/Core/Misc/Feature.cs | Add feature flag for baseBackoffMS support (gated by wire version). |
| src/MongoDB.Driver/Core/Connections/HelloHelper.cs | Emit backpressure: "2" in hello command construction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+22
to
+23
| using System.Threading; | ||
| using System.Threading.Tasks; |
…est comments The baseBackoffMS integration test flaps depending on whether the CI server nightly ships externalClientBaseBackoffMS; the driver behavior is already covered deterministically by the helper, executor, and prose unit tests, so remove it (and the now-unused Feature.ClientBackpressureBaseBackoffMs). Also trim verbose in-method comments across the backpressure tests.
sanych-sun
requested changes
Jul 27, 2026
| public static TimeSpan GetOperationRetryBackoffDelay(int attempt, IRandom random, int? baseBackoffMs = null) | ||
| { | ||
| // Limit a server-supplied override to MaxBackoff so the backoff still resolves to min(MaxBackoff, ...) rather than tripping GetRetryDelayMs's guard. | ||
| var initialBackoff = Math.Min( |
Member
There was a problem hiding this comment.
Why do we need to guard this with Math.Min here? GetRetryDelayMs also do that. Isn't that enough?
| /// or <see langword="null"/> when absent. Only <see cref="MongoCommandException"/> results are inspected; a | ||
| /// missing, non-numeric, or non-positive value is treated as absent. | ||
| /// </summary> | ||
| public static int? GetBaseBackoffMs(Exception exception) |
| [InlineData(2, 20000, 0, 10000)] | ||
| public void GetOperationRetryBackoffDelay_should_apply_baseBackoffMs_override(int attempt, int? baseBackoffMs, int expectedRangeMin, int expectedRangeMax) | ||
| { | ||
| var result = RetryabilityHelper.GetOperationRetryBackoffDelay(attempt, DefaultRandom.Instance, baseBackoffMs); |
Member
There was a problem hiding this comment.
For this test it makes sense provide the mocked IRandom which always returns same value (1?) and expect the strict value.
- GetBaseBackoffMs returns long? and accepts only Int32/Int64 (the server sends baseBackoffMS as a long long / Int64), which also avoids an OverflowException on a Decimal128/NaN/Inf value. - Widen GetRetryDelayMs's backoffInitial to long so the server override is passed through and capped by its own min(), dropping the separate clamp. - Make the override unit test deterministic (mocked IRandom + strict values) and add Decimal128/Double regression cases. - Remove an unused using; repoint the Test 1/3/4 spec links to the merged spec.
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.
https://jira.mongodb.org/browse/CSHARP-6089
Implements the client backpressure
baseBackoffMSspec changes (mongodb/specifications#1953):backpressurefield changes fromtrueto the string"2".baseBackoffMSon an overload error overrides the default backpressure backoff base.2^attemptper the spec (shared with the transactions-convenient-API retry backoff).