Skip to content

Commit e70d74b

Browse files
committed
Improves DB transaction retry logic
do not log if non deadlock exception just throw it
1 parent cd8c033 commit e70d74b

File tree

1 file changed

+16
-15
lines changed

1 file changed

+16
-15
lines changed

src/DBTransactionRetryHelper.php

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,17 @@ class DBTransactionRetryHelper
2424
*/
2525
public static function transactionWithRetry(Closure $callback, int $maxRetries = 3, int $retryDelay = 2, string $logFileName = 'database/mysql-deadlocks', string $trxLabel = ''): mixed
2626
{
27-
if (is_null($trxLabel)){
27+
if (is_null($trxLabel)) {
2828
$trxLabel = '';
2929
}
3030
$attempt = 0;
3131
$log = [];
32-
$isDeadlock = false;
3332

3433
while ($attempt < $maxRetries) {
34+
// reset per-attempt flags to avoid stale values
3535
$throwable = null;
3636
$exceptionCatched = false;
37+
$isDeadlock = false;
3738

3839
try {
3940
// Execute the transaction
@@ -44,46 +45,46 @@ public static function transactionWithRetry(Closure $callback, int $maxRetries =
4445
} catch (QueryException $e) {
4546
$exceptionCatched = true;
4647

47-
// Determine if this is a retryable deadlock/serialization failure
48+
// Only deadlock/serialization *may* be retried and logged
4849
$isDeadlock = static::isDeadlockOrSerializationError($e);
4950

5051
if ($isDeadlock) {
5152
$attempt++;
5253
$log[] = static::buildLogContext($e, $attempt, $maxRetries, $trxLabel);
5354

5455
if ($attempt >= $maxRetries) {
56+
// exhausted retries — throw after logging below in finally
5557
$throwable = $e;
5658
} else {
57-
// Exponential backoff with jitter (minimal change but more robust)
59+
// Exponential backoff with jitter
5860
$delay = static::backoffDelay($retryDelay, $attempt);
5961
sleep($delay);
60-
continue; // retry next loop
62+
continue; // retry
6163
}
6264
} else {
63-
// Non-deadlock exception: propagate
65+
// Non-deadlock: DO NOT log, just rethrow
6466
$throwable = $e;
6567
}
6668
} finally {
6769
if (is_null($throwable) && !$exceptionCatched) {
68-
// Success on the first try; optionally log last attempt as warning
70+
// Success on first try, nothing to do.
71+
// If you want to warn when there WERE previous retries that succeeded, keep this block:
6972
if (count($log) > 0) {
73+
// optional: downgrade to warning for eventual success after retries
7074
generateLog($log[count($log) - 1], $logFileName, 'warning');
7175
}
7276
} elseif (!is_null($throwable)) {
73-
// Ensure non-deadlock exceptions are logged
74-
if (count($log) > 0) {
77+
// We only log when it is a DEADLOCK and retries are exhausted.
78+
if ($isDeadlock && count($log) > 0) {
7579
generateLog($log[count($log) - 1], $logFileName);
76-
} else if (!$isDeadlock && $throwable instanceof QueryException) {
77-
// Log non-deadlock QueryException immediately
78-
$context = static::buildLogContext($throwable, $attempt, $maxRetries, $trxLabel);
79-
generateLog($context, $logFileName);
8080
}
81+
82+
// For NON-deadlock, nothing is logged — just throw.
8183
throw $throwable;
8284
}
8385
}
8486
}
8587

86-
// If we exit the loop without returning, throw a generic runtime exception
8788
throw new \RuntimeException('Transaction with retry exhausted after ' . $maxRetries . ' attempts.');
8889
}
8990

@@ -159,4 +160,4 @@ protected static function backoffDelay(int $baseDelay, int $attempt): int
159160
$max = $delay + $jitter;
160161
return random_int($min, $max);
161162
}
162-
}
163+
}

0 commit comments

Comments
 (0)