From 9f27981b01414a437022120745b8761e461b7945 Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Wed, 13 May 2026 03:19:09 +0200 Subject: [PATCH] feat(database): add transaction retry attempts - Add an optional attempts parameter to transaction(). - Retry whole transactions for RetryableTransactionException failures. - Preserve existing behavior for nested transactions and non-retryable failures. - Document retry behavior, caveats, and side-effect guidance. Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- system/Database/BaseConnection.php | 89 +++++-- system/Database/BasePreparedQuery.php | 6 +- system/Database/ConnectionInterface.php | 3 +- tests/system/Database/BaseConnectionTest.php | 226 ++++++++++++++++++ .../Database/Live/TransactionClosureTest.php | 143 +++++++++++ user_guide_src/source/changelogs/v4.8.0.rst | 2 +- .../source/database/transactions.rst | 57 ++++- .../source/database/transactions/016.php | 7 + 8 files changed, 495 insertions(+), 38 deletions(-) create mode 100644 user_guide_src/source/database/transactions/016.php diff --git a/system/Database/BaseConnection.php b/system/Database/BaseConnection.php index d7f1362050c3..2507c9cddb39 100644 --- a/system/Database/BaseConnection.php +++ b/system/Database/BaseConnection.php @@ -18,6 +18,7 @@ use CodeIgniter\Database\Exceptions\RetryableTransactionException; use CodeIgniter\Database\Exceptions\UniqueConstraintViolationException; use CodeIgniter\Events\Events; +use CodeIgniter\Exceptions\InvalidArgumentException; use CodeIgniter\I18n\Time; use Exception; use ReflectionClass; @@ -228,6 +229,11 @@ abstract class BaseConnection implements ConnectionInterface */ protected ?DatabaseException $lastException = null; + /** + * The first database exception that caused the current transaction to fail. + */ + protected ?DatabaseException $transFailureException = null; + /** * Connection ID * @@ -860,7 +866,7 @@ public function query(string $sql, $binds = null, bool $setEscapeFlags = true, s $query->setDuration($startTime, $startTime); // This will trigger a rollback if transactions are being used - $this->handleTransStatus(); + $this->handleTransStatus($exception ?? $this->lastException); if ( $this->DBDebug @@ -1082,44 +1088,67 @@ public function afterRollback(callable $callback): static * @template TReturn * * @param callable(self): TReturn $callback + * @param positive-int $attempts * * @return false|TReturn */ - public function transaction(callable $callback): mixed + public function transaction(callable $callback, int $attempts = 1): mixed { + if ($attempts < 1) { + throw new InvalidArgumentException('Transaction attempts must be a positive integer.'); + } + if (! $this->transEnabled) { return $callback($this); } - if (! $this->transBegin()) { - return false; - } + $attempts = $this->transDepth === 0 ? $attempts : 1; + + for ($attempt = 1; $attempt <= $attempts; $attempt++) { + if (! $this->transBegin()) { + return false; + } - try { - $result = $callback($this); - } catch (Throwable $e) { try { - $this->transRollback(); - } catch (Throwable $rollbackException) { - log_message('error', 'Database: Transaction callback threw an exception before rollback failed: ' . $e); + $result = $callback($this); + } catch (Throwable $e) { + try { + $this->transRollback(); + } catch (Throwable $rollbackException) { + log_message('error', 'Database: Transaction callback threw an exception before rollback failed: ' . $e); + + throw $rollbackException; + } finally { + if ($this->transDepth > 0) { + $this->transStatus = false; + } elseif ($this->transStrict === false) { + $this->transStatus = true; + } + } - throw $rollbackException; - } finally { - if ($this->transDepth > 0) { - $this->transStatus = false; - } elseif ($this->transStrict === false) { - $this->transStatus = true; + if ($this->transDepth === 0 && $e instanceof RetryableTransactionException && $attempt < $attempts) { + $this->prepareTransactionRetry(); + + continue; } + + throw $e; } - throw $e; - } + if (! $this->transComplete()) { + if ($this->transDepth === 0 && $this->transFailureException instanceof RetryableTransactionException && $attempt < $attempts) { + $this->prepareTransactionRetry(); - if (! $this->transComplete()) { - return false; + continue; + } + + return false; + } + + return $result; } - return $result; + return false; } /** @@ -1145,7 +1174,8 @@ public function transBegin(bool $testMode = false): bool // Reset the transaction failure flag. // If the $testMode flag is set to TRUE transactions will be rolled back // even if the queries produce a successful result. - $this->transFailure = $testMode; + $this->transFailure = $testMode; + $this->transFailureException = null; if ($this->_transBegin()) { $this->transDepth++; @@ -1219,13 +1249,24 @@ public function resetTransStatus(): static * * @internal This method is for internal database component use only */ - public function handleTransStatus(): void + public function handleTransStatus(?DatabaseException $exception = null): void { if ($this->transDepth !== 0) { $this->transStatus = false; + $this->transFailureException ??= $exception; } } + /** + * Reset transaction state that should not leak into a retry attempt. + */ + protected function prepareTransactionRetry(): void + { + $this->transStatus = true; + $this->transFailureException = null; + $this->lastException = null; + } + /** * Run and clear callbacks registered for a successful transaction commit. */ diff --git a/system/Database/BasePreparedQuery.php b/system/Database/BasePreparedQuery.php index 3914c03dc771..cdd8839e5ab8 100644 --- a/system/Database/BasePreparedQuery.php +++ b/system/Database/BasePreparedQuery.php @@ -141,11 +141,11 @@ public function execute(...$data) if ($result === false) { $query->setDuration($startTime, $startTime); - // This will trigger a rollback if transactions are being used - $this->db->handleTransStatus(); - $databaseException = $this->createDatabaseException($exception); + // This will trigger a rollback if transactions are being used + $this->db->handleTransStatus($databaseException); + if ($this->db->DBDebug) { // We call this function in order to roll-back queries // if transactions are enabled. If we don't call this here diff --git a/system/Database/ConnectionInterface.php b/system/Database/ConnectionInterface.php index 0a46099a2087..6991bd089472 100644 --- a/system/Database/ConnectionInterface.php +++ b/system/Database/ConnectionInterface.php @@ -144,10 +144,11 @@ public function afterRollback(callable $callback): static; * @template TReturn * * @param callable(self): TReturn $callback + * @param positive-int $attempts * * @return false|TReturn */ - public function transaction(callable $callback): mixed; + public function transaction(callable $callback, int $attempts = 1): mixed; /** * Returns an instance of the query builder for this connection. diff --git a/tests/system/Database/BaseConnectionTest.php b/tests/system/Database/BaseConnectionTest.php index adf922ecd07f..a679fd5a54f2 100644 --- a/tests/system/Database/BaseConnectionTest.php +++ b/tests/system/Database/BaseConnectionTest.php @@ -14,10 +14,13 @@ namespace CodeIgniter\Database; use CodeIgniter\Database\Exceptions\DatabaseException; +use CodeIgniter\Exceptions\InvalidArgumentException; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockConnection; +use ErrorException; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; +use Tests\Support\Mock\MockPreparedQuery; use Throwable; use TypeError; @@ -713,6 +716,224 @@ protected function _transBegin(): bool $this->assertFalse($callbackRan); } + public function testTransactionRejectsInvalidAttempts(): void + { + $db = new MockConnection($this->options); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Transaction attempts must be a positive integer.'); + + $db->transaction(static function (): void {}, attempts: self::invalidTransactionAttempts()); + } + + public function testTransactionRetriesSuppressedRetryableQueryFailure(): void + { + $db = new class ($this->options) extends MockConnection { + public int $queries = 0; + + public function query(string $sql, $binds = null, bool $setEscapeFlags = true, string $queryClass = ''): bool + { + $this->queries++; + + if ($this->queries === 1) { + $exception = $this->createDatabaseException('Deadlock found when trying to get lock.', 1213); + + $this->setLastException($exception); + $this->handleTransStatus($exception); + + return false; + } + + return true; + } + + protected function isRetryableTransactionErrorCode(int|string $code): bool + { + return $code === 1213; + } + }; + + $callbackRuns = 0; + + $result = $db->transaction(static function (BaseConnection $connection) use (&$callbackRuns): string|false { + $callbackRuns++; + $result = $connection->query('INSERT INTO job (name) VALUES (\'Retried Job\')'); + + return $result === true ? 'committed' : false; + }, attempts: 2); + + $this->assertSame('committed', $result); + $this->assertSame(2, $callbackRuns); + $this->assertSame(2, $db->queries); + $this->assertNotInstanceOf(DatabaseException::class, $db->getLastException()); + } + + public function testTransactionRetriesRetryableQueryFailureWhenTransExceptionRollsBack(): void + { + $db = new class ($this->options) extends MockConnection { + public int $queries = 0; + + public function query(string $sql, $binds = null, bool $setEscapeFlags = true, string $queryClass = '') + { + return BaseConnection::query($sql, $binds, $setEscapeFlags, $queryClass); + } + + protected function execute(string $sql): object + { + $this->queries++; + + if ($this->queries === 1) { + throw $this->createDatabaseException('Deadlock found when trying to get lock.', 1213); + } + + return (object) []; + } + + protected function isRetryableTransactionErrorCode(int|string $code): bool + { + return $code === 1213; + } + }; + $db->transException(true); + + $callbackRuns = 0; + + $result = $db->transaction(static function (BaseConnection $connection) use (&$callbackRuns): string { + $callbackRuns++; + $connection->query('INSERT INTO job (name) VALUES (\'Retried Job\')'); + + return 'committed'; + }, attempts: 2); + + $this->assertSame('committed', $result); + $this->assertSame(2, $callbackRuns); + $this->assertSame(2, $db->queries); + $this->assertSame(0, $db->transDepth); + } + + public function testTransactionDoesNotRetrySuppressedNonRetryableQueryFailure(): void + { + $db = new class ($this->options) extends MockConnection { + public int $queries = 0; + + public function query(string $sql, $binds = null, bool $setEscapeFlags = true, string $queryClass = ''): bool + { + $this->queries++; + $this->handleTransStatus($this->createDatabaseException('Syntax error.', 1064)); + + return false; + } + }; + + $callbackRuns = 0; + + $result = $db->transaction(static function (BaseConnection $connection) use (&$callbackRuns): string|false { + $callbackRuns++; + $result = $connection->query('INSERT INTO job (name) VALUES (\'Failed Job\')'); + + return $result === true ? 'not returned' : false; + }, attempts: 2); + + $this->assertFalse($result); + $this->assertSame(1, $callbackRuns); + $this->assertSame(1, $db->queries); + } + + public function testTransactionRetriesSuppressedRetryablePreparedQueryFailure(): void + { + $db = new class (array_merge($this->options, ['DBDebug' => false])) extends MockConnection { + protected function isRetryableTransactionErrorCode(int|string $code): bool + { + return $code === 1213; + } + }; + + $callbackRuns = 0; + $preparedQuery = new MockPreparedQuery($db); + $preparedQuery->prepare('INSERT INTO job (name) VALUES (?)'); + + $result = $db->transaction(static function () use (&$callbackRuns, $preparedQuery): bool { + $callbackRuns++; + $preparedQuery->thrownException = $callbackRuns === 1 + ? new ErrorException('Deadlock found when trying to get lock.', 1213) + : null; + + return $preparedQuery->execute('Retried Job'); + }, attempts: 2); + + $this->assertTrue($result); + $this->assertSame(2, $callbackRuns); + } + + public function testTransactionDoesNotRetryCallbackExceptionWhenRollbackFails(): void + { + $db = new class ($this->options) extends MockConnection { + protected function _transRollback(): bool + { + return false; + } + + protected function isRetryableTransactionErrorCode(int|string $code): bool + { + return $code === 1213; + } + }; + + $callbackRuns = 0; + + try { + $db->transaction(static function (BaseConnection $connection) use (&$callbackRuns): void { + $callbackRuns++; + + throw $connection->createDatabaseException('Deadlock found when trying to get lock.', 1213); + }, attempts: 2); + $this->fail('Expected retryable transaction exception.'); + } catch (DatabaseException $e) { + $this->assertSame('Deadlock found when trying to get lock.', $e->getMessage()); + } + + $this->assertSame(1, $callbackRuns); + $this->assertSame(1, $db->transDepth); + } + + public function testTransactionDoesNotRetrySuppressedQueryFailureWhenRollbackFails(): void + { + $db = new class ($this->options) extends MockConnection { + public int $queries = 0; + + public function query(string $sql, $binds = null, bool $setEscapeFlags = true, string $queryClass = ''): bool + { + $this->queries++; + $this->handleTransStatus($this->createDatabaseException('Deadlock found when trying to get lock.', 1213)); + + return false; + } + + protected function _transRollback(): bool + { + return false; + } + + protected function isRetryableTransactionErrorCode(int|string $code): bool + { + return $code === 1213; + } + }; + + $callbackRuns = 0; + + $result = $db->transaction(static function (BaseConnection $connection) use (&$callbackRuns): bool { + $callbackRuns++; + + return $connection->query('INSERT INTO job (name) VALUES (\'Failed Job\')'); + }, attempts: 2); + + $this->assertFalse($result); + $this->assertSame(1, $callbackRuns); + $this->assertSame(1, $db->queries); + $this->assertSame(1, $db->transDepth); + } + public function testTransactionRunsCallbackWhenTransactionsAreDisabled(): void { $db = new MockConnection($this->options); @@ -751,4 +972,9 @@ protected function getDriverFunctionPrefix(): string $this->assertTrue($db->callFunction('contains', 'CodeIgniter', 'Ignite')); } + + private static function invalidTransactionAttempts(): int + { + return 0; + } } diff --git a/tests/system/Database/Live/TransactionClosureTest.php b/tests/system/Database/Live/TransactionClosureTest.php index 8e869a943461..a52b331d0712 100644 --- a/tests/system/Database/Live/TransactionClosureTest.php +++ b/tests/system/Database/Live/TransactionClosureTest.php @@ -14,6 +14,7 @@ namespace CodeIgniter\Database\Live; use CodeIgniter\Database\BaseConnection; +use CodeIgniter\Database\Exceptions\RetryableTransactionException; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\DatabaseTestTrait; use Config\Database; @@ -71,6 +72,125 @@ public function testTransactionReturnsCallbackResultAndCommits(): void $this->seeInDatabase('job', ['name' => 'Committed Job']); } + public function testTransactionRetriesRetryableExceptionAndCommits(): void + { + $attempts = 0; + $callbacks = []; + + $result = $this->db->transaction(static function (BaseConnection $db) use (&$attempts, &$callbacks): string { + $attempts++; + + $db->table('job')->insert([ + 'name' => 'Retried Job ' . $attempts, + 'description' => 'Only the final attempt should commit.', + ]); + $db->afterCommit(static function () use (&$callbacks, &$attempts): void { + $callbacks[] = $attempts; + }); + + if ($attempts === 1) { + throw new RetryableTransactionException('Deadlock found when trying to get lock.', 1213); + } + + return 'committed'; + }, attempts: 2); + + $this->assertSame('committed', $result); + $this->assertSame(2, $attempts); + $this->assertSame([2], $callbacks); + $this->dontSeeInDatabase('job', ['name' => 'Retried Job 1']); + $this->seeInDatabase('job', ['name' => 'Retried Job 2']); + } + + public function testRollbackCallbacksRunForFailedRetryAttempts(): void + { + $attempts = 0; + $callbacks = []; + + $result = $this->db->transaction(static function (BaseConnection $db) use (&$attempts, &$callbacks): string { + $attempts++; + + $db->afterRollback(static function () use (&$callbacks, &$attempts): void { + $callbacks[] = $attempts; + }); + + if ($attempts === 1) { + throw new RetryableTransactionException('Deadlock found when trying to get lock.', 1213); + } + + return 'committed'; + }, attempts: 2); + + $this->assertSame('committed', $result); + $this->assertSame(2, $attempts); + $this->assertSame([1], $callbacks); + } + + public function testRollbackCallbackExceptionStopsRetryAttempts(): void + { + $attempts = 0; + + try { + $this->db->transaction(static function (BaseConnection $db) use (&$attempts): void { + $attempts++; + + $db->afterRollback(static function (): void { + throw new RuntimeException('Rollback callback failed.'); + }); + + throw new RetryableTransactionException('Deadlock found when trying to get lock.', 1213); + }, attempts: 2); + $this->fail('Expected rollback callback exception.'); + } catch (RuntimeException $e) { + $this->assertSame('Rollback callback failed.', $e->getMessage()); + } + + $this->assertSame(1, $attempts); + } + + public function testTransactionRethrowsRetryableExceptionAfterAttemptsAreExhausted(): void + { + $attempts = 0; + + try { + $this->db->transaction(static function (BaseConnection $db) use (&$attempts): void { + $attempts++; + + $db->table('job')->insert([ + 'name' => 'Rolled Back Retried Job ' . $attempts, + 'description' => 'Every attempt should roll back.', + ]); + + throw new RetryableTransactionException('Deadlock found when trying to get lock.', 1213); + }, attempts: 2); + $this->fail('Expected retryable transaction exception.'); + } catch (RetryableTransactionException $e) { + $this->assertSame('Deadlock found when trying to get lock.', $e->getMessage()); + } + + $this->assertSame(2, $attempts); + $this->dontSeeInDatabase('job', ['name' => 'Rolled Back Retried Job 1']); + $this->dontSeeInDatabase('job', ['name' => 'Rolled Back Retried Job 2']); + } + + public function testTransactionDoesNotRetryNonRetryableException(): void + { + $attempts = 0; + + try { + $this->db->transaction(static function () use (&$attempts): void { + $attempts++; + + throw new RuntimeException('Transaction callback failed.'); + }, attempts: 3); + $this->fail('Expected transaction callback exception.'); + } catch (RuntimeException $e) { + $this->assertSame('Transaction callback failed.', $e->getMessage()); + } + + $this->assertSame(1, $attempts); + } + public function testTransactionPassesConnectionToCallback(): void { $result = $this->db->transaction(fn (BaseConnection $db): bool => $db === $this->db); @@ -292,6 +412,29 @@ public function testNestedTransactionCallbackExceptionMarksOuterTransactionForRo $this->dontSeeInDatabase('job', ['name' => 'Inner Job']); } + public function testNestedTransactionRetryAttemptsRunOnce(): void + { + $attempts = 0; + + $this->db->transStart(); + + try { + $this->db->transaction(static function () use (&$attempts): void { + $attempts++; + + throw new RetryableTransactionException('Deadlock found when trying to get lock.', 1213); + }, attempts: 3); + $this->fail('Expected retryable transaction exception.'); + } catch (RetryableTransactionException) { + // Nested transactions cannot be safely replayed without savepoints. + } + + $this->assertSame(1, $attempts); + $this->assertFalse($this->db->transStatus()); + + $this->db->transComplete(); + } + public function testAfterCommitCallbackExceptionBubblesAfterTransactionCommit(): void { try { diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst index 39fbc6199549..2cff476fe6c4 100644 --- a/user_guide_src/source/changelogs/v4.8.0.rst +++ b/user_guide_src/source/changelogs/v4.8.0.rst @@ -211,7 +211,7 @@ Database - Added ``inTransaction()`` to database connections to check whether the connection is inside an active CodeIgniter-managed transaction. See :ref:`transactions-checking-transaction-state`. - Prepared query execution failures now throw or store typed database exceptions such as ``UniqueConstraintViolationException`` and ``RetryableTransactionException`` when applicable, matching normal query failures. - Added ``RetryableTransactionException`` for driver-specific retryable transaction failures such as deadlocks and serialization failures. See :ref:`transactions-retryable-exceptions`. -- Added the ``transaction()`` method to database connections to run a callback inside a transaction. See :ref:`transactions-closure`. +- Added the ``transaction()`` method to database connections to run a callback inside a transaction, with optional retry attempts for retryable transaction failures. See :ref:`transactions-closure`. - Added ``trustServerCertificate`` option to ``SQLSRV`` database connections in ``Config\Database``. Set it to ``true`` to trust the server certificate without CA validation when using encrypted connections. Query Builder diff --git a/user_guide_src/source/database/transactions.rst b/user_guide_src/source/database/transactions.rst index f98a1ec60473..9f7b0157f2d2 100644 --- a/user_guide_src/source/database/transactions.rst +++ b/user_guide_src/source/database/transactions.rst @@ -72,7 +72,7 @@ If transactions are disabled, ``transaction()`` does not start a transaction. It runs the callback and returns the callback result. If the callback throws an exception, ``transaction()`` rolls back and rethrows -the original exception. +the original exception unless retry attempts remain, as described below. If an ``afterRollback()`` callback throws while ``transaction()`` is rolling back, that callback exception bubbles to the caller instead of the normal @@ -82,6 +82,44 @@ Callbacks registered with ``afterCommit()`` or ``afterRollback()`` inside the transaction callback follow the same rules as other transaction callbacks: they run only after the outermost transaction commits or rolls back. +Retrying Transactions +--------------------- + +You may pass ``attempts`` when you want CodeIgniter to retry the whole +transaction after retryable database concurrency failures, such as deadlocks or +serialization failures. Retries happen when a ``RetryableTransactionException`` +occurs while the callback is running, including from query or prepared query +execution: + +.. literalinclude:: transactions/016.php + +``attempts`` is the total number of times to try the transaction, including the +first run, and must be greater than or equal to ``1``. + +Keep these rules in mind when using retry attempts: + +- The callback may run more than once, and retries run immediately without delay + or backoff. +- Retries only apply when ``transaction()`` starts the outermost transaction. If + ``transaction()`` is called inside an active transaction, the callback runs + once using the existing nested transaction behavior. +- Only ``RetryableTransactionException`` failures are retried. Other exceptions + are rolled back and rethrown without another attempt. +- Retry attempts do not retry failures that are reported only while the + transaction is committing. +- If an ``afterRollback()`` callback throws while a failed attempt is rolling + back, that callback exception bubbles to the caller and no further attempts are + made. + +Avoid non-transactional side effects inside callbacks that may be retried. For +side effects such as queued jobs, emails, cache invalidation, events, or external +API calls, register them with ``afterCommit()`` so they run only after the final +successful commit. + +``afterRollback()`` callbacks may run for failed retry attempts even when a later +attempt commits successfully, so use them only for cleanup that is safe to run +per rolled-back attempt. + .. _transactions-retryable-exceptions: Classifying Retryable Transaction Failures @@ -91,19 +129,20 @@ Classifying Retryable Transaction Failures Some database engines report transaction failures that may succeed when the entire transaction is attempted again, such as deadlocks or serialization -failures. When a driver classifies a query execution failure as one of these -retryable transaction failures, CodeIgniter throws +failures. When a driver classifies a query or prepared query execution failure as +one of these retryable transaction failures, CodeIgniter throws ``RetryableTransactionException`` so you can decide how your application should respond: .. literalinclude:: transactions/015.php -This exception is only a classifier. CodeIgniter does not retry the transaction -automatically. If you retry, run the whole transaction again. Avoid -non-transactional side effects inside transaction bodies that may be retried. For -side effects such as queued jobs, emails, cache invalidation, or external API -calls, register them with ``afterCommit()`` so they run only after the transaction -commits. +``RetryableTransactionException`` is the classifier used by ``transaction()`` +retry attempts. If you are not using ``transaction()`` attempts, catch this +exception and retry the whole transaction according to your application's policy. +Avoid non-transactional side effects inside transaction bodies that may be +retried. For side effects such as queued jobs, emails, cache invalidation, or +external API calls, register them with ``afterCommit()`` so they run only after +the transaction commits. When ``DBDebug`` is ``false`` and a failed query or prepared query returns ``false`` instead of throwing, inspect ``getLastException()`` immediately after diff --git a/user_guide_src/source/database/transactions/016.php b/user_guide_src/source/database/transactions/016.php new file mode 100644 index 000000000000..0ed15f615363 --- /dev/null +++ b/user_guide_src/source/database/transactions/016.php @@ -0,0 +1,7 @@ +db->transaction(static function ($db) use ($order) { + $db->table('orders')->insert($order); + + return $db->insertID(); +}, attempts: 3);