From 3bc94520f853bbe80fc3be1497dd218466d0d2fb Mon Sep 17 00:00:00 2001 From: DariusIII Date: Fri, 14 Aug 2026 10:50:56 +0200 Subject: [PATCH] Update bulk inserts handling --- app/Services/Binaries/BinaryHandler.php | 8 +- app/Services/Binaries/CollectionHandler.php | 8 +- .../Binaries/HeaderStorageService.php | 16 +--- .../Binaries/HeaderStorageTransaction.php | 3 +- app/Services/Binaries/PartHandler.php | 4 +- app/Support/SqlError.php | 74 +++++++++++++++++++ .../Feature/BinariesStorageInternalsTest.php | 20 +++++ tests/Unit/Support/SqlErrorTest.php | 66 +++++++++++++++++ 8 files changed, 174 insertions(+), 25 deletions(-) create mode 100644 app/Support/SqlError.php create mode 100644 tests/Unit/Support/SqlErrorTest.php diff --git a/app/Services/Binaries/BinaryHandler.php b/app/Services/Binaries/BinaryHandler.php index 7b35cde9e..8ceb82395 100644 --- a/app/Services/Binaries/BinaryHandler.php +++ b/app/Services/Binaries/BinaryHandler.php @@ -4,9 +4,9 @@ declare(strict_types=1); namespace App\Services\Binaries; +use App\Support\SqlError; use App\Support\Utf8; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Log; /** * Handles binary record creation and updates during header storage. @@ -86,7 +86,7 @@ final class BinaryHandler } catch (\Throwable $e) { $this->lastException = $e; if (config('app.debug') === true) { - Log::error('Binary insert failed: '.$e->getMessage()); + SqlError::logFailure('Binary insert failed', $e); } } @@ -158,7 +158,7 @@ final class BinaryHandler } catch (\Throwable $e) { $this->lastException = $e; if (config('app.debug') === true) { - Log::error('Bulk binary insert failed: '.$e->getMessage()); + SqlError::logFailure('Bulk binary insert failed', $e); } } @@ -458,7 +458,7 @@ final class BinaryHandler } catch (\Throwable $e) { $this->lastException = $e; if (config('app.debug') === true) { - Log::error('Binaries aggregate update failed: '.$e->getMessage()); + SqlError::logFailure('Binaries aggregate update failed', $e); } return false; diff --git a/app/Services/Binaries/CollectionHandler.php b/app/Services/Binaries/CollectionHandler.php index 80e0f1b65..78303da0c 100644 --- a/app/Services/Binaries/CollectionHandler.php +++ b/app/Services/Binaries/CollectionHandler.php @@ -8,9 +8,9 @@ use App\Enums\CollectionFileCheckStatus; use App\Models\Collection; use App\Services\CollectionsCleaningService; use App\Services\XrefService; +use App\Support\SqlError; use App\Support\Utf8; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Log; /** * Handles collection record creation and retrieval during header storage. @@ -131,7 +131,7 @@ final class CollectionHandler } catch (\Throwable $e) { $this->lastException = $e; if (config('app.debug') === true) { - Log::error('Collection insert failed: '.$e->getMessage()); + SqlError::logFailure('Collection insert failed', $e); } } @@ -233,7 +233,7 @@ final class CollectionHandler } catch (\Throwable $e) { $this->lastException = $e; if (config('app.debug') === true) { - Log::error('Bulk collection insert failed: '.$e->getMessage()); + SqlError::logFailure('Bulk collection insert failed', $e); } } @@ -676,7 +676,7 @@ final class CollectionHandler } catch (\Throwable $e) { $this->lastException = $e; if (config('app.debug') === true) { - Log::error('Collection aggregate refresh failed: '.$e->getMessage()); + SqlError::logFailure('Collection aggregate refresh failed', $e); } return false; diff --git a/app/Services/Binaries/HeaderStorageService.php b/app/Services/Binaries/HeaderStorageService.php index b8a2ded04..6603c99ec 100644 --- a/app/Services/Binaries/HeaderStorageService.php +++ b/app/Services/Binaries/HeaderStorageService.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace App\Services\Binaries; -use Illuminate\Database\QueryException; +use App\Support\SqlError; /** * Orchestrates the header storage process. @@ -180,19 +180,7 @@ final class HeaderStorageService private function isTransientLockError(?\Throwable $exception): bool { - if ($exception === null) { - return false; - } - if ($exception instanceof QueryException) { - $driverCode = (int) ($exception->errorInfo[1] ?? 0); - if ($exception->getCode() === '40001' || \in_array($driverCode, [1205, 1213], true)) { - return true; - } - } - - return str_contains($exception->getMessage(), 'Deadlock found') - || str_contains($exception->getMessage(), 'Lock wait timeout exceeded') - || str_contains($exception->getMessage(), 'database is locked'); + return $exception !== null && SqlError::isTransientLock($exception); } /** diff --git a/app/Services/Binaries/HeaderStorageTransaction.php b/app/Services/Binaries/HeaderStorageTransaction.php index 209b723b3..4b701c316 100644 --- a/app/Services/Binaries/HeaderStorageTransaction.php +++ b/app/Services/Binaries/HeaderStorageTransaction.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Services\Binaries; +use App\Support\SqlError; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; @@ -87,7 +88,7 @@ final class HeaderStorageTransaction $this->rollbackAndCleanup(); if (config('app.debug') === true) { - Log::error('HeaderStorageTransaction commit failed: '.$e->getMessage()); + SqlError::logFailure('HeaderStorageTransaction commit failed', $e); } return false; diff --git a/app/Services/Binaries/PartHandler.php b/app/Services/Binaries/PartHandler.php index b20725744..c85c9754f 100644 --- a/app/Services/Binaries/PartHandler.php +++ b/app/Services/Binaries/PartHandler.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace App\Services\Binaries; +use App\Support\SqlError; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Log; /** * Handles part record creation during header storage. @@ -176,7 +176,7 @@ final class PartHandler } catch (\Throwable $e) { $this->lastException = $e; if (config('app.debug') === true) { - Log::error('Parts chunk insert failed: '.$e->getMessage()); + SqlError::logFailure('Parts chunk insert failed', $e); } return null; diff --git a/app/Support/SqlError.php b/app/Support/SqlError.php new file mode 100644 index 000000000..683961407 --- /dev/null +++ b/app/Support/SqlError.php @@ -0,0 +1,74 @@ +errorInfo[1] ?? 0); + if ($exception->getCode() === '40001' || \in_array($driverCode, self::TRANSIENT_DRIVER_CODES, true)) { + return true; + } + } + + return str_contains($exception->getMessage(), 'Deadlock found') + || str_contains($exception->getMessage(), 'Lock wait timeout exceeded') + || str_contains($exception->getMessage(), 'Record has changed since last read') + || str_contains($exception->getMessage(), 'database is locked'); + } + + /** + * Concise single-line description without the "(Connection: …, SQL: …)" + * payload Laravel appends to QueryException messages. + */ + public static function describe(\Throwable $exception): string + { + $message = $exception->getMessage(); + $tail = strpos($message, ' (Connection:'); + if ($tail !== false) { + $message = substr($message, 0, $tail); + } + + if (mb_strlen($message) > self::MAX_MESSAGE_LENGTH) { + $message = mb_substr($message, 0, self::MAX_MESSAGE_LENGTH).'…'; + } + + return $message; + } + + /** + * Log a failed statement: warning for transient lock conflicts (the caller + * retries the chunk), error otherwise. Always without the SQL payload. + */ + public static function logFailure(string $context, \Throwable $exception): void + { + $message = $context.': '.self::describe($exception); + if (self::isTransientLock($exception)) { + Log::warning($message.' (transient, chunk will be retried)'); + } else { + Log::error($message); + } + } +} diff --git a/tests/Feature/BinariesStorageInternalsTest.php b/tests/Feature/BinariesStorageInternalsTest.php index f58a7ecea..e495981c6 100644 --- a/tests/Feature/BinariesStorageInternalsTest.php +++ b/tests/Feature/BinariesStorageInternalsTest.php @@ -11,6 +11,7 @@ use App\Services\Binaries\HeaderStorageTransaction; use App\Services\Binaries\PartHandler; use App\Services\BlacklistService; use App\Services\CollectionsCleaningService; +use Illuminate\Database\QueryException; use Illuminate\Support\Facades\DB; use Tests\TestCase; @@ -390,6 +391,25 @@ class BinariesStorageInternalsTest extends TestCase )'); } + public function test_record_changed_since_last_read_is_treated_as_transient(): void + { + $service = new HeaderStorageService($this->deterministicCollectionHandler(), config: new BinariesConfig(sqlChunkSize: 10)); + $method = new \ReflectionMethod($service, 'isTransientLockError'); + + $pdo1020 = new \PDOException("SQLSTATE[HY000]: General error: 1020 Record has changed since last read in table 'collections'; try restarting transaction"); + $pdo1020->errorInfo = ['HY000', 1020, "Record has changed since last read in table 'collections'"]; + $checkRead = new QueryException('mariadb', 'INSERT INTO collections ...', [], $pdo1020); + + $this->assertTrue($method->invoke($service, $checkRead)); + + $pdo1062 = new \PDOException('SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry'); + $pdo1062->errorInfo = ['23000', 1062, 'Duplicate entry']; + $duplicateKey = new QueryException('mariadb', 'INSERT INTO collections ...', [], $pdo1062); + + $this->assertFalse($method->invoke($service, $duplicateKey)); + $this->assertFalse($method->invoke($service, null)); + } + private function deterministicCollectionHandler(): CollectionHandler { return new CollectionHandler(new class extends CollectionsCleaningService diff --git a/tests/Unit/Support/SqlErrorTest.php b/tests/Unit/Support/SqlErrorTest.php new file mode 100644 index 000000000..0c015fbfd --- /dev/null +++ b/tests/Unit/Support/SqlErrorTest.php @@ -0,0 +1,66 @@ +errorInfo = ['HY000', 1020, "Record has changed since last read in table 'collections'"]; + + $this->assertTrue(SqlError::isTransientLock(new QueryException('mariadb', 'INSERT ...', [], $pdo))); + } + + public function test_deadlock_and_lock_wait_timeout_are_transient(): void + { + foreach ([1213, 1205] as $code) { + $pdo = new \PDOException("SQLSTATE[40001]: Serialization failure: {$code}"); + $pdo->errorInfo = ['40001', $code, 'Serialization failure']; + + $this->assertTrue(SqlError::isTransientLock(new QueryException('mariadb', 'INSERT ...', [], $pdo))); + } + } + + public function test_duplicate_key_is_not_transient(): void + { + $pdo = new \PDOException('SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry'); + $pdo->errorInfo = ['23000', 1062, 'Duplicate entry']; + + $this->assertFalse(SqlError::isTransientLock(new QueryException('mariadb', 'INSERT ...', [], $pdo))); + $this->assertFalse(SqlError::isTransientLock(new \RuntimeException('unrelated failure'))); + } + + public function test_message_fallback_detects_transient_plain_exceptions(): void + { + $this->assertTrue(SqlError::isTransientLock(new \RuntimeException('Deadlock found when trying to get lock'))); + $this->assertTrue(SqlError::isTransientLock(new \RuntimeException('Lock wait timeout exceeded'))); + $this->assertTrue(SqlError::isTransientLock(new \RuntimeException("Record has changed since last read in table 'collections'"))); + $this->assertTrue(SqlError::isTransientLock(new \RuntimeException('database is locked'))); + } + + public function test_describe_strips_connection_and_sql_payload(): void + { + $pdo = new \PDOException('SQLSTATE[40001]: Serialization failure: 1213 Deadlock found when trying to get lock; try restarting transaction'); + $pdo->errorInfo = ['40001', 1213, 'Deadlock found when trying to get lock; try restarting transaction']; + $queryException = new QueryException('mariadb', 'INSERT INTO collections VALUES (?)', ['binary-garbage'], $pdo); + + $description = SqlError::describe($queryException); + + $this->assertStringContainsString('Deadlock found', $description); + $this->assertStringNotContainsString('(Connection:', $description); + $this->assertStringNotContainsString('binary-garbage', $description); + } + + public function test_describe_truncates_overlong_messages(): void + { + $description = SqlError::describe(new \RuntimeException(str_repeat('x', 1000))); + + $this->assertLessThanOrEqual(501, mb_strlen($description)); + $this->assertStringEndsWith('…', $description); + } +}