PYTHON-5885 Client Backpressure with baseBackoffMS#2877
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Note: SERVER-130142 will rename |
There was a problem hiding this comment.
Pull request overview
Adds support for using retryAfterMS from retryable overload errors to influence the client’s adaptive retry backoff, plus updates handshake backpressure metadata and related tests/docs.
Changes:
- Parse and propagate
retryAfterMSfrom server error documents into the overload backoff calculation. - Update hello/handshake backpressure field value (and metadata tests) to
"2". - Add a new prose-style timing test validating that
retryAfterMSreduces overload backoff.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
pymongo/errors.py |
Parses retryAfterMS from error details into PyMongoError. |
pymongo/synchronous/mongo_client.py |
Uses retryAfterMS override when computing overload retry backoff (sync). |
pymongo/asynchronous/mongo_client.py |
Uses retryAfterMS override when computing overload retry backoff (async). |
pymongo/synchronous/helpers.py |
Adjusts backoff helper to accept an override base backoff (sync). |
pymongo/asynchronous/helpers.py |
Adjusts backoff helper to accept an override base backoff (async). |
pymongo/synchronous/pool.py |
Updates handshake hello backpressure value (sync). |
pymongo/asynchronous/pool.py |
Updates handshake hello backpressure value (async). |
test/test_client_backpressure.py |
Adds a new test for retryAfterMS-driven backoff behavior (sync). |
test/asynchronous/test_client_backpressure.py |
Adds a new test for retryAfterMS-driven backoff behavior (async). |
test/test_client_metadata.py |
Updates handshake backpressure assertion (sync). |
test/asynchronous/test_client_metadata.py |
Updates handshake backpressure assertion (async). |
doc/changelog.rst |
Updates changelog entry for 4.18.0. |
| def __init__( | ||
| self, | ||
| message: str = "", | ||
| error_labels: Optional[Iterable[str]] = None, | ||
| base_backoff_ms: Optional[int] = None, | ||
| ) -> None: | ||
| super().__init__(message) | ||
| self._message = message | ||
| self._error_labels = set(error_labels or []) | ||
| self._base_backoff_ms = base_backoff_ms |
There was a problem hiding this comment.
Why does the base PyMongoError also need to initialize self._base_backoff_ms? It's only a viable label on OperationFailure.
|
|
| if overloaded: | ||
| self._max_retries = self._client.options.max_adaptive_retries | ||
| always_retryable = exc.has_error_label("RetryableError") and overloaded | ||
| self._base_backoff_ms = exc_to_check._base_backoff_ms |
There was a problem hiding this comment.
🤖 suggests:
self._base_backoff_ms = getattr(exc_to_check, "_base_backoff_ms", None)
here and here because _base_backoff_ms is a property defined only on OperationFailure but exc_to_check can be a ConnectionFailure and the if overloaded: guard only checks the error label not the type.
There was a problem hiding this comment.
The server can only attach SystemOverloadedError to OperationFailure errors, so this can't ever occur currently. We could add the getattr as a safeguard in case that changes in the future, but it also makes that existing contract with the server less clear.
Actually, never mind. Having to add a type override here instead of getattr is silly.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
doc/changelog.rst:5
- The changelog header uses a placeholder release date "2026/XX/XX". This is not a valid date and makes the changelog look released/dated when it isn’t; consider keeping the undated header until the release date is known.
Changes in Version 4.18.0 (2026/XX/XX)
--------------------------------------
doc/changelog.rst:13
- This changelog bullet is a single very long line; wrap it to match the surrounding formatting and avoid exceeding typical reStructuredText line-length conventions.
- Improved performance for MongoDB 9.0's Intelligent Workload Management (IWM) by only retrying overload errors when doing so is expected to not worsen server conditions.
test/asynchronous/test_client_backpressure.py:306
- The PR description says it adds prose test
test_05_overload_errors_with_retryafterms_override_backoff, but the added test here istest_05_overload_errors_with_basebackoffms_override_backoff. Please update the PR description (or rename the test) so they match.
@async_client_context.require_version_min(9, 0, 0, -1)
@async_client_context.require_failCommand_appName
async def test_05_overload_errors_with_basebackoffms_override_backoff(self, random_func):
# Drivers should test that overload errors with `baseBackoffMS` override the default backoff duration.
PYTHON-5885
Changes in this PR
Adds
baseBackoffMSsupport for backoff calculation on retryable overload errors.Test Plan
Adds a new prose test
test_05_overload_errors_with_retryafterms_override_backoffto verify thebaseBackoffMSbehavior. Note that this new test requires a server withbaseBackoffMSsupport.Checklist
Checklist for Author
[ ] Is any followup work tracked in a JIRA ticket? If so, add link(s).Checklist for Reviewer