PYTHON-5752 - Only retry overload errors if retries are enabled#2726
PYTHON-5752 - Only retry overload errors if retries are enabled#2726NoahStapp merged 7 commits intomongodb:backpressurefrom
Conversation
NoahStapp
requested review from
ShaneHarvey,
caseyclements and
sleepyStick
and removed request for
a team
|
Encryption test failures are expected on backpressure. |
| self._client.options.retry_reads and self._client.options.retry_writes | ||
| ): | ||
| raise | ||
| if self._is_aggregate_write and not self._client.options.retry_writes: |
There was a problem hiding this comment.
Is this correct? aggregate writes are never consider a retryable read or write (hence why retryable=False was passed previously). They should only be retried on overload errors. Do we even need the is_aggregate_write flag at all?
There was a problem hiding this comment.
All aggregate operations, including writes, call retryable_read. This causes them to be treated as reads even though retryWrites determines whether or not overload error retries are allowed. This check here is for that specific case.
There was a problem hiding this comment.
Is there a test that makes sure aggregate + writes are NOT retried when they hit retryable write errors?
There was a problem hiding this comment.
For non-overload retryable write errors?
There was a problem hiding this comment.
Yes. Things like ShutdownInProgress that include the RetryableWriteError label.
There was a problem hiding this comment.
| @@ -2010,6 +2009,8 @@ async def _retry_internal( | |||
| read_pref: Optional[_ServerMode] = None, | |||
There was a problem hiding this comment.
The encryption errors seem relevant. Is this expected?
[2026/03/10 10:26:26.362] FAILURE: AttributeError: AsyncMongoClient has no attribute '_retry_policy'. To access the _retry_policy database, use client['_retry_policy']. ()
[2026/03/10 10:26:26.362] self = <test.asynchronous.test_encryption.TestDeadlockProse testMethod=test_case_5>
[2026/03/10 10:26:26.362] async def test_case_5(self):
[2026/03/10 10:26:26.362] > await self._run_test(
[2026/03/10 10:26:26.362] max_pool_size=None,
[2026/03/10 10:26:26.362] auto_encryption_opts=AutoEncryptionOpts(
[2026/03/10 10:26:26.362] *self.optargs, bypass_auto_encryption=False, key_vault_client=None
[2026/03/10 10:26:26.362] ),
[2026/03/10 10:26:26.362] )
[2026/03/10 10:26:26.362] test/asynchronous/test_encryption.py:1605:
[2026/03/10 10:26:26.362] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
[2026/03/10 10:26:26.362] test/asynchronous/test_encryption.py:1504: in _run_test
[2026/03/10 10:26:26.362] client_encrypted = await self.async_rs_or_single_client(
[2026/03/10 10:26:26.362] test/asynchronous/__init__.py:1127: in async_rs_or_single_client
[2026/03/10 10:26:26.362] return await self._async_mongo_client(h, p, **kwargs)
[2026/03/10 10:26:26.362] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[2026/03/10 10:26:26.362] test/asynchronous/__init__.py:1035: in _async_mongo_client
[2026/03/10 10:26:26.362] client = AsyncMongoClient(uri, port, **client_options)
[2026/03/10 10:26:26.362] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[2026/03/10 10:26:26.362] pymongo/asynchronous/mongo_client.py:897: in __init__
[2026/03/10 10:26:26.362] self._init_based_on_options(self._seeds, srv_max_hosts, srv_service_name)
[2026/03/10 10:26:26.362] pymongo/asynchronous/mongo_client.py:1007: in _init_based_on_options
[2026/03/10 10:26:26.362] self._encrypter = _Encrypter(self, self._options.auto_encryption_opts)
[2026/03/10 10:26:26.362] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[2026/03/10 10:26:26.362] pymongo/asynchronous/encryption.py:434: in __init__
[2026/03/10 10:26:26.362] key_vault_coll = key_vault_client[db][coll]
[2026/03/10 10:26:26.362] ^^^^^^^^^^^^^^^^^^^^
[2026/03/10 10:26:26.362] pymongo/asynchronous/mongo_client.py:1366: in __getitem__
[2026/03/10 10:26:26.362] return database.AsyncDatabase(self, name)
[2026/03/10 10:26:26.362] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[2026/03/10 10:26:26.362] pymongo/asynchronous/database.py:138: in __init__
[2026/03/10 10:26:26.362] self._retry_policy = client._retry_policy
[2026/03/10 10:26:26.362] ^^^^^^^^^^^^^^^^^^^^There was a problem hiding this comment.
Ah, this must have been hidden by the expected encryption errors previously. We just need to instantiate self._retry_policy before we call self._init_based_on_options in the client constructor.
| } | ||
| }, | ||
| { | ||
| "connectionCheckedInEvent": {} |
There was a problem hiding this comment.
Is the order between commandStartedEvent/commandFailedEvent and connectionCheckedInEvent well defined in our specs?
It might be safer to only check connectionCheckOutEvent and connectionCheckedInEvent.
NoahStapp
changed the title
3 participants
PYTHON-5752
Changes in this PR
Adherence to the backpressure spec. Overload errors are only retried if the appropriate
retryWrite/retryRead=Trueoption is set for the client (both are by default).runCommandexplicitly requires bothretryReadsandretryWritesto be enabled in order to retry overload errors.aggregateis normally a read operation, except when the pipeline contains an$out/$mergestage, in which case it is treated as a write.Test Plan
Added the tests in DRIVERS-3416.
Checklist
Checklist for Author
Checklist for Reviewer