PYTHON-5794 - Add prose tests to verify correct retry behavior when a…#2755
PYTHON-5794 - Add prose tests to verify correct retry behavior when a…#2755NoahStapp merged 16 commits intomongodb:masterfrom
Conversation
… mix of overload and non-overload errors are encountered
There was a problem hiding this comment.
Pull request overview
Adds new prose-style tests and updates client retry logic to ensure that, after encountering a SystemOverloadedError, subsequent retryable (non-overload) errors can continue to use the increased retry budget (per the backpressure spec intent referenced in PYTHON-5794/DRIVERS-3446).
Changes:
- Add new prose tests for “overload then non-overload” sequences for retryable reads and writes (sync + async).
- Update
_ClientConnectionRetryableretry eligibility to track a per-operation_max_retriesbudget that expands when an overload error is observed. - Add helper plumbing in retryable reads tests to synchronously reconfigure fail points from a
CommandFailedEventcallback.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_retryable_writes.py | Adds a new prose test asserting increased write retries after overload then non-overload errors. |
| test/test_retryable_reads.py | Adds setup client + helper to reconfigure fail points from events, and a new prose test for reads. |
| test/asynchronous/test_retryable_writes.py | Async equivalent of the new write prose test. |
| test/asynchronous/test_retryable_reads.py | Async equivalent of the reads helper + new prose test. |
| pymongo/synchronous/mongo_client.py | Adjusts retry logic to expand retry budget after overload errors and enforce via _max_retries. |
| pymongo/asynchronous/mongo_client.py | Async equivalent of the retry budget changes. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
OCSP test failures are unrelated. |
| listener.failed = failed | ||
|
|
||
| client = await self.async_rs_client(event_listeners=[listener]) | ||
| await client.test.test.insert_one({}) |
There was a problem hiding this comment.
NIT should we make test.test part of every test's setup in the future so we don't have to keep remembering to do it here?
There was a problem hiding this comment.
What do you mean by test.test?
There was a problem hiding this comment.
I mean client.test.test.insert_one() could just be in every setup function so new code can be tested without this esoteric complication muddying things up.
There was a problem hiding this comment.
I don't think we want to add an additional database operation to every test setup if the test doesn't need the insert_one.
| def setUp(self) -> None: | ||
| super().setUp() | ||
| self.setup_client = MongoClient(**client_context.client_options) | ||
| self.addCleanup(self.setup_client.close) | ||
|
|
There was a problem hiding this comment.
(Separate PR) Can we add creating the collection here or do we have several deviations?
There was a problem hiding this comment.
Can you open a ticket detailing what you mean?
There was a problem hiding this comment.
Absolutely. https://jira.mongodb.org/browse/PYTHON-5796
Co-authored-by: Jib <Jibzade@gmail.com>
… mix of overload and non-overload errors are encountered
PYTHON-5794
Changes in this PR
Test Plan
Added two new prose tests.
Checklist
Checklist for Author
Checklist for Reviewer