From 8126a5c1282288bdfe482d79835c31d28640054f Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 3 Nov 2026 22:31:54 +0100 Subject: [PATCH] fix(backups): limit S3 uploads to 1000 parts for providers like Scaleway Closes #8411 Co-Authored-By: Claude Opus 5.5 --- app/Jobs/DatabaseBackupJob.php | 5 ++-- app/Jobs/VolumeBackupJob.php | 10 +++---- app/Models/S3Storage.php | 27 +++++++++++++++++++ ...tabaseBackupMultipleS3DestinationsTest.php | 23 ++++++++++++++++ ...VolumeBackupMultipleS3DestinationsTest.php | 18 +++++++++++++ tests/Unit/S3StorageTest.php | 12 +++++++++ 6 files changed, 88 insertions(+), 7 deletions(-) diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index 1ebf176e1f..b72a1ee8d4 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -450,7 +450,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue throw new \Exception('Unsupported database type'); } - $size = $this->calculate_size(); + $size = $this->size = (int) $this->calculate_size(); // Verify local backup succeeded if ($size > 0) { @@ -960,8 +960,9 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue $escapedBackupLocation = escapeshellarg($this->backup_location); $escapedS3Destination = escapeshellarg("temporary/{$bucket}{$this->backup_dir}/"); $resolveOptions = $s3->minioClientResolveOptions($this->server, $fullImageName, $network); + $partSizeOption = S3Storage::minioClientPartSizeOption($this->size); $this->setMinioClientAlias($this->server, "backup-of-{$this->backup_log_uuid}", $s3, $resolveOptions); - $uploadCommand = "docker exec backup-of-{$this->backup_log_uuid} mc cp{$resolveOptions} {$escapedBackupLocation} {$escapedS3Destination}"; + $uploadCommand = "docker exec {$partSizeOption} backup-of-{$this->backup_log_uuid} mc cp{$resolveOptions} {$escapedBackupLocation} {$escapedS3Destination}"; } S3Storage::retryMinioClientUpload(fn () => instant_remote_process( [$uploadCommand], diff --git a/app/Jobs/VolumeBackupJob.php b/app/Jobs/VolumeBackupJob.php index 0216c8ee4a..d27632982e 100644 --- a/app/Jobs/VolumeBackupJob.php +++ b/app/Jobs/VolumeBackupJob.php @@ -178,7 +178,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue } elseif ($this->backup->save_s3) { $s3CleanupPending = true; $this->execution->update(['s3_cleanup_pending' => true]); - $uploadErrors = $this->uploadToDestinations($backupLocation, $backupDirectory, $server); + $uploadErrors = $this->uploadToDestinations($backupLocation, $size, $backupDirectory, $server); $s3Uploaded = $uploadErrors === []; if ($s3Uploaded) { @@ -439,7 +439,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue * * @return array One error message per failed destination. */ - private function uploadToDestinations(string $backupLocation, string $backupDirectory, Server $server): array + private function uploadToDestinations(string $backupLocation, int $size, string $backupDirectory, Server $server): array { $errors = []; @@ -450,7 +450,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue throw new \RuntimeException('The selected S3 storage no longer exists.'); } - $this->uploadToS3($replica->s3, $backupLocation, $backupDirectory, $server); + $this->uploadToS3($replica->s3, $backupLocation, $size, $backupDirectory, $server); $replica->update(['s3_uploaded' => true, 'message' => null]); } catch (Throwable $exception) { $message = $this->redact($exception->getMessage()); @@ -464,7 +464,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue return $errors; } - private function uploadToS3(S3Storage $s3, string $backupLocation, string $backupDirectory, Server $server): void + private function uploadToS3(S3Storage $s3, string $backupLocation, int $size, string $backupDirectory, Server $server): void { $s3->testConnection(shouldSave: true); $containerName = 'volume-upload-'.$this->execution->uuid; @@ -481,7 +481,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue } else { $resolveOptions = $s3->minioClientResolveOptions($server, $image); $this->setMinioClientAlias($server, $containerName, $s3, $resolveOptions); - $uploadCommand = 'docker exec '.escapeshellarg($containerName).' mc cp'.$resolveOptions.' '.escapeshellarg($backupLocation).' ' + $uploadCommand = 'docker exec '.S3Storage::minioClientPartSizeOption($size).' '.escapeshellarg($containerName).' mc cp'.$resolveOptions.' '.escapeshellarg($backupLocation).' ' .escapeshellarg('temporary/'.$s3->bucket.$backupDirectory.'/'); } S3Storage::retryMinioClientUpload(fn () => instant_remote_process([$uploadCommand], $server, timeout: $this->timeout, disableMultiplexing: true)); diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index 9c76104ac2..ba1d33a587 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -41,6 +41,18 @@ class S3Storage extends BaseModel */ private const MINIO_CLIENT_UPLOAD_RETRY_DELAYS_MS = [10_000, 30_000]; + /** + * The most parts that an upload with the MinIO client uses. Most providers accept 10000 parts, but some, like + * Scaleway, accept only 1000. The MinIO client sizes the parts for 10000 parts (16 MiB for files up to about + * 156 GiB), so without a part size, files larger than about 15.6 GiB fail on these providers. + */ + private const MINIO_CLIENT_MAX_UPLOAD_PARTS = 1000; + + /** + * The smallest part size of an upload with the MinIO client. It is also the default part size of the MinIO client. + */ + private const MINIO_CLIENT_MIN_PART_SIZE_BYTES = 16 * 1024 * 1024; + /** * How long the presigned URL of a curl transfer is valid. S3 checks it when the request starts, so it must * cover the wait for the earlier transfers and the retries of the backup job. @@ -335,6 +347,21 @@ class S3Storage extends BaseModel .($this->skip_tls_verification ? ' -e MC_INSECURE=true' : ''); } + /** + * The `docker exec` option that makes `mc cp` upload a file of $fileSize bytes in at most 1000 parts. + * `mc cp` has no part size flag, but it reads the part size from MC_UPLOAD_MULTIPART_SIZE. + */ + public static function minioClientPartSizeOption(int $fileSize): string + { + $mebibyte = 1024 * 1024; + $partSize = max( + self::MINIO_CLIENT_MIN_PART_SIZE_BYTES, + (int) ceil($fileSize / self::MINIO_CLIENT_MAX_UPLOAD_PARTS / $mebibyte) * $mebibyte, + ); + + return '-e MC_UPLOAD_MULTIPART_SIZE='.$partSize; + } + /** * Path-style URLs (`endpoint/bucket`) are the default. Providers like Alibaba Cloud OSS only accept * virtual-hosted-style URLs (`bucket.endpoint`). A storage that is not saved yet has no value, so it uses the default. diff --git a/tests/Feature/DatabaseBackupMultipleS3DestinationsTest.php b/tests/Feature/DatabaseBackupMultipleS3DestinationsTest.php index 4acf47a048..8542b9519f 100644 --- a/tests/Feature/DatabaseBackupMultipleS3DestinationsTest.php +++ b/tests/Feature/DatabaseBackupMultipleS3DestinationsTest.php @@ -344,6 +344,29 @@ describe('backup job', function () { expect($regionOfBucket)->toBe(['first-bucket' => 'ru-central-1', 'second-bucket' => 'us-east-1']); }); + it('sizes the upload parts of a large dump for providers that allow only 1000 parts', function () { + $backup = createMultiS3Backup(); + S3Storage::query()->update(['endpoint' => 'https://8.8.8.8']); + $disk = Mockery::mock(FilesystemAdapter::class); + $disk->shouldReceive('files')->andReturn([]); + $sshDisk = Storage::disk('ssh-keys'); + Storage::shouldReceive('disk')->with('ssh-keys')->andReturn($sshDisk); + Storage::shouldReceive('build')->andReturn($disk); + $commands = collect(); + Process::fake(function ($process) use ($commands) { + $commands->push(remoteProcessScript($process)); + + return Process::result(output: str_contains(remoteProcessScript($process), 'du -b') ? (string) (19 * 1024 * 1024 * 1024) : ''); + }); + + (new DatabaseBackupJob($backup))->handle(); + + $uploads = $commands->filter(fn (string $command) => str_contains($command, ' mc cp ')); + expect(ScheduledDatabaseBackupExecution::query()->sole()->s3_uploaded)->toBeTrue() + ->and($uploads)->toHaveCount(2) + ->and($uploads->every(fn (string $command) => str_contains($command, 'docker exec -e MC_UPLOAD_MULTIPART_SIZE='.(20 * 1024 * 1024).' backup-of-')))->toBeTrue(); + }); + it('keeps the local file and names the failed destination when one upload fails', function () { [$execution, $commands] = runMultiS3BackupJob(['Second']); diff --git a/tests/Feature/VolumeBackupMultipleS3DestinationsTest.php b/tests/Feature/VolumeBackupMultipleS3DestinationsTest.php index ae6826f584..4faa5f0df7 100644 --- a/tests/Feature/VolumeBackupMultipleS3DestinationsTest.php +++ b/tests/Feature/VolumeBackupMultipleS3DestinationsTest.php @@ -92,6 +92,24 @@ function fakeMultiDestinationS3Disks(): void Storage::shouldReceive('build')->zeroOrMoreTimes()->andReturn($disk); } +it('sizes the upload parts of a large archive for providers that allow only 1000 parts', function () { + [$backup] = createMultiDestinationVolumeBackup(); + fakeMultiDestinationS3Disks(); + fakeRemoteProcesses([ + '*du -b*' => (string) (19 * 1024 * 1024 * 1024), + '*' => '', + ]); + + (new VolumeBackupJob($backup))->handle(); + + expect(ScheduledVolumeBackupExecution::query()->sole()->s3_uploaded)->toBeTrue(); + foreach (['first-bucket', 'second-bucket'] as $bucket) { + Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), 'docker exec -e MC_UPLOAD_MULTIPART_SIZE='.(20 * 1024 * 1024).' ') + && str_contains(remoteProcessScript($process), ' mc cp ') + && str_contains(remoteProcessScript($process), "temporary/{$bucket}/")); + } +}); + it('archives locally and uploads to every destination instead of streaming', function () { [$backup, $first, $second] = createMultiDestinationVolumeBackup(['disable_local_backup' => true]); fakeMultiDestinationS3Disks(); diff --git a/tests/Unit/S3StorageTest.php b/tests/Unit/S3StorageTest.php index 5718dadad4..d653b8380e 100644 --- a/tests/Unit/S3StorageTest.php +++ b/tests/Unit/S3StorageTest.php @@ -299,6 +299,18 @@ test('S3Storage does not retry a MinIO client upload that timed out', function ( Sleep::assertNeverSlept(); }); +test('S3Storage sizes MinIO client upload parts for at most 1000 parts', function (int $fileSize, int $partSize) { + expect(S3Storage::minioClientPartSizeOption($fileSize))->toBe('-e MC_UPLOAD_MULTIPART_SIZE='.$partSize) + ->and((int) ceil($fileSize / $partSize))->toBeLessThanOrEqual(1000); +})->with([ + 'empty file keeps the 16 MiB default' => [0, 16 * 1024 * 1024], + 'small file keeps the 16 MiB default' => [128, 16 * 1024 * 1024], + '1000 parts of 16 MiB keep the default' => [1000 * 16 * 1024 * 1024, 16 * 1024 * 1024], + 'one byte more needs 17 MiB parts' => [1000 * 16 * 1024 * 1024 + 1, 17 * 1024 * 1024], + '19 GiB dump' => [19 * 1024 * 1024 * 1024, 20 * 1024 * 1024], + '1 TiB dump' => [1024 * 1024 * 1024 * 1024, 1049 * 1024 * 1024], +]); + test('S3Storage uses curl transfers only for endpoints with a path, which the MinIO client rejects', function (string $endpoint, bool $expected) { $s3Storage = new S3Storage; $s3Storage->setRawAttributes(['endpoint' => $endpoint]);