diff --git a/pymongo/asynchronous/client_session.py b/pymongo/asynchronous/client_session.py index ed19f16f4..31e6ceb38 100644 --- a/pymongo/asynchronous/client_session.py +++ b/pymongo/asynchronous/client_session.py @@ -467,6 +467,7 @@ class _Transaction: self.sharded = False self.recovery_token = None self.attempt = 0 + self.has_completed_command = False def __del__(self) -> None: if self.conn_mgr: diff --git a/pymongo/synchronous/client_session.py b/pymongo/synchronous/client_session.py index 64056f33c..3165dd52b 100644 --- a/pymongo/synchronous/client_session.py +++ b/pymongo/synchronous/client_session.py @@ -465,6 +465,7 @@ class _Transaction: self.sharded = False self.recovery_token = None self.attempt = 0 + self.has_completed_command = False def __del__(self) -> None: if self.conn_mgr: diff --git a/test/asynchronous/test_retryable_writes.py b/test/asynchronous/test_retryable_writes.py index d1568b3ec..97f99dc16 100644 --- a/test/asynchronous/test_retryable_writes.py +++ b/test/asynchronous/test_retryable_writes.py @@ -732,7 +732,6 @@ class TestErrorPropagationAfterEncounteringMultipleErrors(AsyncIntegrationTest): async def test_03_drivers_return_the_correct_error_when_receiving_some_errors_with_NoWritesPerformed_and_some_without_NoWritesPerformed( self ) -> None: - # TODO: read the expected behavior and add breakpoint() to the retry loop # Create a client with retryWrites=true. listener = OvertCommandListener() @@ -761,7 +760,7 @@ class TestErrorPropagationAfterEncounteringMultipleErrors(AsyncIntegrationTest): } def failed(event: CommandFailedEvent) -> None: - # Configure the fail point command only if the the failed event is for the 91 error configured in step 2. + # Configure the fail point command only if the failed event is for the 91 error configured in step 2. if listener.failed_events: return assert event.failure["code"] == 91 diff --git a/test/asynchronous/test_transactions.py b/test/asynchronous/test_transactions.py index 7b28b5bd9..2376a1af2 100644 --- a/test/asynchronous/test_transactions.py +++ b/test/asynchronous/test_transactions.py @@ -20,6 +20,7 @@ import random import sys import time from io import BytesIO +from unittest.mock import patch import pymongo from gridfs.asynchronous.grid_file import AsyncGridFS, AsyncGridFSBucket @@ -654,66 +655,57 @@ class TestTransactionsConvenientAPI(AsyncTransactionsBase): async def test_4_retry_backoff_is_enforced(self): client = async_client_context.client coll = client[self.db.name].test - # patch random to make it deterministic -- once to effectively have - # no backoff and the second time with "max" backoff (always waiting the longest - # possible time) - _original_random_random = random.random + end = start = no_backoff_time = 0 - def always_one(): - return 1 + # Make random.random always return 0 (no backoff) + with patch.object(random, "random", return_value=0): + # set fail point to trigger transaction failure and trigger backoff + await self.set_fail_point( + { + "configureFailPoint": "failCommand", + "mode": {"times": 13}, + "data": { + "failCommands": ["commitTransaction"], + "errorCode": 251, + }, + } + ) + self.addAsyncCleanup( + self.set_fail_point, {"configureFailPoint": "failCommand", "mode": "off"} + ) - def always_zero(): - return 0 + async def callback(session): + await coll.insert_one({}, session=session) - random.random = always_zero - # set fail point to trigger transaction failure and trigger backoff - await self.set_fail_point( - { - "configureFailPoint": "failCommand", - "mode": {"times": 13}, - "data": { - "failCommands": ["commitTransaction"], - "errorCode": 251, - }, - } - ) - self.addAsyncCleanup( - self.set_fail_point, {"configureFailPoint": "failCommand", "mode": "off"} - ) + start = time.monotonic() + async with self.client.start_session() as s: + await s.with_transaction(callback) + end = time.monotonic() + no_backoff_time = end - start - async def callback(session): - await coll.insert_one({}, session=session) - - start = time.monotonic() - async with self.client.start_session() as s: - await s.with_transaction(callback) - end = time.monotonic() - no_backoff_time = end - start - - random.random = always_one - # set fail point to trigger transaction failure and trigger backoff - await self.set_fail_point( - { - "configureFailPoint": "failCommand", - "mode": { - "times": 13 - }, # sufficiently high enough such that the time effect of backoff is noticeable - "data": { - "failCommands": ["commitTransaction"], - "errorCode": 251, - }, - } - ) - self.addAsyncCleanup( - self.set_fail_point, {"configureFailPoint": "failCommand", "mode": "off"} - ) - start = time.monotonic() - async with self.client.start_session() as s: - await s.with_transaction(callback) - end = time.monotonic() - self.assertLess(abs(end - start - (no_backoff_time + 2.2)), 1) # sum of 13 backoffs is 2.2 - - random.random = _original_random_random + # Make random.random always return 1 (max backoff) + with patch.object(random, "random", return_value=1): + # set fail point to trigger transaction failure and trigger backoff + await self.set_fail_point( + { + "configureFailPoint": "failCommand", + "mode": { + "times": 13 + }, # sufficiently high enough such that the time effect of backoff is noticeable + "data": { + "failCommands": ["commitTransaction"], + "errorCode": 251, + }, + } + ) + self.addAsyncCleanup( + self.set_fail_point, {"configureFailPoint": "failCommand", "mode": "off"} + ) + start = time.monotonic() + async with self.client.start_session() as s: + await s.with_transaction(callback) + end = time.monotonic() + self.assertLess(abs(end - start - (no_backoff_time + 2.2)), 1) # sum of 5 backoffs is 2.2 class TestOptionsInsideTransactionProse(AsyncTransactionsBase): diff --git a/test/test_retryable_writes.py b/test/test_retryable_writes.py index 8763c89de..4daa5643c 100644 --- a/test/test_retryable_writes.py +++ b/test/test_retryable_writes.py @@ -728,7 +728,6 @@ class TestErrorPropagationAfterEncounteringMultipleErrors(IntegrationTest): def test_03_drivers_return_the_correct_error_when_receiving_some_errors_with_NoWritesPerformed_and_some_without_NoWritesPerformed( self ) -> None: - # TODO: read the expected behavior and add breakpoint() to the retry loop # Create a client with retryWrites=true. listener = OvertCommandListener() @@ -757,7 +756,7 @@ class TestErrorPropagationAfterEncounteringMultipleErrors(IntegrationTest): } def failed(event: CommandFailedEvent) -> None: - # Configure the fail point command only if the the failed event is for the 91 error configured in step 2. + # Configure the fail point command only if the failed event is for the 91 error configured in step 2. if listener.failed_events: return assert event.failure["code"] == 91 diff --git a/test/test_transactions.py b/test/test_transactions.py index 861d6d0c0..0159d9231 100644 --- a/test/test_transactions.py +++ b/test/test_transactions.py @@ -20,6 +20,7 @@ import random import sys import time from io import BytesIO +from unittest.mock import patch import pymongo from gridfs.synchronous.grid_file import GridFS, GridFSBucket @@ -642,62 +643,57 @@ class TestTransactionsConvenientAPI(TransactionsBase): def test_4_retry_backoff_is_enforced(self): client = client_context.client coll = client[self.db.name].test - # patch random to make it deterministic -- once to effectively have - # no backoff and the second time with "max" backoff (always waiting the longest - # possible time) - _original_random_random = random.random + end = start = no_backoff_time = 0 - def always_one(): - return 1 + # Make random.random always return 0 (no backoff) + with patch.object(random, "random", return_value=0): + # set fail point to trigger transaction failure and trigger backoff + self.set_fail_point( + { + "configureFailPoint": "failCommand", + "mode": {"times": 13}, + "data": { + "failCommands": ["commitTransaction"], + "errorCode": 251, + }, + } + ) + self.addCleanup( + self.set_fail_point, {"configureFailPoint": "failCommand", "mode": "off"} + ) - def always_zero(): - return 0 + def callback(session): + coll.insert_one({}, session=session) - random.random = always_zero - # set fail point to trigger transaction failure and trigger backoff - self.set_fail_point( - { - "configureFailPoint": "failCommand", - "mode": {"times": 13}, - "data": { - "failCommands": ["commitTransaction"], - "errorCode": 251, - }, - } - ) - self.addCleanup(self.set_fail_point, {"configureFailPoint": "failCommand", "mode": "off"}) + start = time.monotonic() + with self.client.start_session() as s: + s.with_transaction(callback) + end = time.monotonic() + no_backoff_time = end - start - def callback(session): - coll.insert_one({}, session=session) - - start = time.monotonic() - with self.client.start_session() as s: - s.with_transaction(callback) - end = time.monotonic() - no_backoff_time = end - start - - random.random = always_one - # set fail point to trigger transaction failure and trigger backoff - self.set_fail_point( - { - "configureFailPoint": "failCommand", - "mode": { - "times": 13 - }, # sufficiently high enough such that the time effect of backoff is noticeable - "data": { - "failCommands": ["commitTransaction"], - "errorCode": 251, - }, - } - ) - self.addCleanup(self.set_fail_point, {"configureFailPoint": "failCommand", "mode": "off"}) - start = time.monotonic() - with self.client.start_session() as s: - s.with_transaction(callback) - end = time.monotonic() - self.assertLess(abs(end - start - (no_backoff_time + 2.2)), 1) # sum of 13 backoffs is 2.2 - - random.random = _original_random_random + # Make random.random always return 1 (max backoff) + with patch.object(random, "random", return_value=1): + # set fail point to trigger transaction failure and trigger backoff + self.set_fail_point( + { + "configureFailPoint": "failCommand", + "mode": { + "times": 13 + }, # sufficiently high enough such that the time effect of backoff is noticeable + "data": { + "failCommands": ["commitTransaction"], + "errorCode": 251, + }, + } + ) + self.addCleanup( + self.set_fail_point, {"configureFailPoint": "failCommand", "mode": "off"} + ) + start = time.monotonic() + with self.client.start_session() as s: + s.with_transaction(callback) + end = time.monotonic() + self.assertLess(abs(end - start - (no_backoff_time + 2.2)), 1) # sum of 5 backoffs is 2.2 class TestOptionsInsideTransactionProse(TransactionsBase):