PYTHON-5505 Prototype system overload retry loop for all operations #2497

ShaneHarvey · 2025-08-19T22:35:03Z

PYTHON-5505 Prototype system overload retry loop for all operations

Description:
All operations that fail with the "Retryable" error label will be retried up to 3 times. When the last failure includes the "SystemOverloaded" error label we apply exponential backoff with jitter before attempting a retry.

PR Tasks:

Validate command path
Validate retryable write path
Validate retryable write path with an otherwise non-retryable operation (eg update_many).
Validate retryable read path
Validate cursor getMore path

Passing patch build: https://spruce.mongodb.com/version/68a64f28268fea00074a72a4/tasks

…d on overload The server only adds the "Retryable" error label when it knows it is safe to retry.

ShaneHarvey · 2025-08-19T23:15:19Z

test/asynchronous/test_backpressure.py

+    RUN_ON_LOAD_BALANCER = True
+
+    @async_client_context.require_failCommand_appName
+    async def test_retry_overload_error_command(self):


Note these scenarios will be later turned into unified spec tests.

NoahStapp · 2025-08-20T14:07:36Z

pymongo/asynchronous/mongo_client.py

@@ -2783,14 +2786,19 @@ async def run(self) -> T:
                # most likely be a waste of time.
                raise
            except PyMongoError as exc:
+                overload = False


For my own understanding: what's the purpose of this overload value? It's True when the error is an overload error?

that's my understanding of it -- and then when overload is true and we've reached max_retries, we raise the error, otherwise we backoff and try again. (I think)

And after this initial attempt, we only ever set overload to exc.error.has_error_label("Retryable"), is there a reason why we cant just set it to that here?

(side note: I like how _overload and _retrying are the same length, but I think _overloaded is a slightly more accurate var? i don't think this matters that much though since I feel like _overload does get the point across for me)

I think _overload is only set when the error is a result of the server being overloaded. If so, _overloaded is a clearer variable name.

That's all correct. I've renamed it overloaded.

Update: this was renamed "always_retryable". "overloaded" is now based on the "SystemOverloaded" error label.

ShaneHarvey · 2025-08-20T18:52:48Z

test_retry_overload_error_update_many fails on actions but passes locally. I'm investigating...

____________ TestBackpressure.test_retry_overload_error_update_many ____________

self = <test.asynchronous.test_backpressure.TestBackpressure testMethod=test_retry_overload_error_update_many>

    @async_client_context.require_failCommand_appName
    async def test_retry_overload_error_update_many(self):
        # Even though update_many is not a retryable write operation, it will
        # still be retried via the "Retryable" error label.
        await self.db.t.insert_one({"x": 1})
    
        # Ensure command is retried on overload error.
        fail_once = mock_overload_error.copy()
        fail_once["mode"] = {"times": _MAX_RETRIES}
        async with self.fail_point(fail_once):
>           await self.db.t.update_many({}, {"$set": {"x": 2}})

https://github.com/mongodb/mongo-python-driver/actions/runs/17106265661/job/48515851763?pr=2497

pymongo/synchronous/mongo_client.py

NoahStapp · 2025-08-20T20:51:05Z

pymongo/asynchronous/mongo_client.py

-                        retryable_write_error_exc = isinstance(
-                            exc.error, PyMongoError
-                        ) and exc.error.has_error_label("RetryableWriteError")
+                        if isinstance(exc.error, PyMongoError):


Do we need this if-else block? Both branches are identical.

Yeah this can be cleaned up. I this it's a problem with ClientBulkWriteException violating Liskov substitution principle. Perhaps ClientBulkWriteException should override has_error_label() to behave correctly. I suspect the NoWritesPerformed check below is buggy because of this issue too.

I opened: https://jira.mongodb.org/browse/PYTHON-5511

Refactored to remove the duplicate label checks. I decided not to do PYTHON-5511 yet to reduce the scope of this PR.

NoahStapp · 2025-08-20T20:52:50Z

pymongo/asynchronous/mongo_client.py

@@ -2783,14 +2786,21 @@ async def run(self) -> T:
                # most likely be a waste of time.
                raise
            except PyMongoError as exc:
+                always_retryable = False


always_retryable represents what exactly? That an error is retryable under all conditions?

Yes it means it's safe to retry even if the operation is otherwise non-retryable, like an update_many. There's probably a better name for it.

overloaded still takes precedence over always_retryable, correct?

…re not supported

ShaneHarvey · 2025-08-20T22:49:14Z

Found the bug causing the update_many test to fail. This was only happening on standalone nodes which don't support retryable writes. The fix is to skip the check for sessions_supported when attempting to retry after a system overload error. I believe this is safe.

Passing patch build: https://spruce.mongodb.com/version/68a64f28268fea00074a72a4/tasks

NoahStapp · 2025-08-21T13:50:30Z

Found the bug causing the update_many test to fail. This was only happening on standalone nodes which don't support retryable writes. The fix is to skip the check for sessions_supported when attempting to retry after a system overload error. I believe this is safe.

Passing patch build: spruce.mongodb.com/version/68a64f28268fea00074a72a4/tasks

We can't retry writes on standalone anyway, so how does skipping the sessions_supported check fix that? Wouldn't that cause us to attempt a retry when we can't?

ShaneHarvey · 2025-08-21T15:20:25Z

We can't retry writes on standalone anyway, so how does skipping the sessions_supported check fix that? Wouldn't that cause us to attempt a retry when we can't?

This feature adds the ability to retry all commands that fail with the "Retryable" error label, even on standalone. The server only adds this label when it's safe to retry (eg the command was rejected before doing any work).

ShaneHarvey added 3 commits August 19, 2025 15:08

PYTHON-5505 Prototype system overload retry loop for all operations

4728868

PYTHON-5505 Make cursor getMore retryable

a83dea8

PYTHON-5505 Use functools.wraps to fix test_comment

c2ff971

ShaneHarvey marked this pull request as ready for review August 19, 2025 23:03

ShaneHarvey requested a review from a team as a code owner August 19, 2025 23:03

ShaneHarvey requested review from NoahStapp and sleepyStick August 19, 2025 23:03

PYTHON-5505 Add test that even non-retryable write ops will be retrie…

b6dd34d

…d on overload The server only adds the "Retryable" error label when it knows it is safe to retry.

ShaneHarvey commented Aug 19, 2025

View reviewed changes

NoahStapp reviewed Aug 20, 2025

View reviewed changes

PYTHON-5505 Rename overloaded flag

eb113f0

ShaneHarvey requested a review from NoahStapp August 20, 2025 17:53

PYTHON-5505 cleanup tests

b436647

gracicot reviewed Aug 20, 2025

View reviewed changes

pymongo/synchronous/mongo_client.py Outdated Show resolved Hide resolved

ShaneHarvey added 4 commits August 20, 2025 12:36

PYTHON-5505 Only apply backoff when SystemOverloaded label is present

f3be7f7

PYTHON-5505 Fix _retry_overload handling of backoff too

65fbd7e

PYTHON-5505 Remove unused "no_retry" flag

194863f

PYTHON-5505 Fix RetryableWriteError label check

989fdfe

NoahStapp requested changes Aug 20, 2025

View reviewed changes

ShaneHarvey added 3 commits August 20, 2025 14:41

PYTHON-5505 Fix retry eligibility bug

1f40e11

PYTHON-5505 Fix handling of ClientBulkWriteException

6114788

PYTHON-5505 Allow retry write after overload error even if sessions a…

d2e3354

…re not supported

ShaneHarvey requested a review from NoahStapp August 20, 2025 23:21

NoahStapp approved these changes Aug 21, 2025

View reviewed changes

ShaneHarvey merged commit 75eee91 into mongodb:backpressure Aug 21, 2025
31 checks passed

PYTHON-5505 Prototype system overload retry loop for all operations #2497

PYTHON-5505 Prototype system overload retry loop for all operations #2497

Uh oh!

Conversation

ShaneHarvey commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NoahStapp commented Aug 21, 2025

Uh oh!

ShaneHarvey commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShaneHarvey commented Aug 19, 2025 •

edited

Loading

ShaneHarvey commented Aug 20, 2025 •

edited

Loading

ShaneHarvey Aug 20, 2025 •

edited

Loading

ShaneHarvey commented Aug 20, 2025 •

edited

Loading

ShaneHarvey commented Aug 21, 2025 •

edited

Loading