From a9b6b9271d9cb40b6806dbc5402b38aac89d8499 Mon Sep 17 00:00:00 2001 From: slach Date: Wed, 9 Sep 2026 20:18:35 +0500 Subject: [PATCH 1/3] Fail fast on missing remote objects and add allow_missing_files_on_download Backuper.Classify retried every non-context error, so a permanently missing remote object (S3 NoSuchKey/404, GCS 404, Azure BlobNotFound, FTP/SFTP not-found) burned the whole retries_on_failure x retries_duration backoff per file. storage.IsNotFoundErr now recognises these across backends (typed SDK errors plus the string markers that used to live in isRemoteMetadataNotFound) and Classify returns Fail. Add general.allow_missing_files_on_download (ALLOW_MISSING_FILES_ON_DOWNLOAD, default false), --allow-missing-files for download/restore_remote and the allow_missing_files API query argument: a salvage mode which skips data parts missing on remote storage with an error-level log and a final summary, drops them from the local table metadata (parts + files) and lets the intact parts of a partially corrupted backup be restored. Metadata files are never skipped; upload_by_part=false bundles stay fatal because a single part can't be carved out of a shared archive. Fix https://github.com/Altinity/clickhouse-backup/issues/1456 Co-Authored-By: Claude Fable 5.1 --- ChangeLog.md | 1 + ReadMe.md | 5 + cmd/clickhouse-backup/main.go | 10 ++ pkg/backup/backuper.go | 8 ++ pkg/backup/backuper_test.go | 4 + pkg/backup/download.go | 135 ++++++++++++++---- pkg/backup/download_test.go | 21 --- pkg/config/config.go | 10 ++ pkg/server/server.go | 10 ++ pkg/storage/not_found_error.go | 65 +++++++++ pkg/storage/not_found_error_test.go | 52 +++++++ .../integration/testAllowMissingFiles_test.go | 114 +++++++++++++++ test/integration/testMetadataNotFound_test.go | 2 +- .../tests/snapshots/cli.py.cli.snapshot | 2 +- 14 files changed, 388 insertions(+), 51 deletions(-) create mode 100644 pkg/storage/not_found_error.go create mode 100644 pkg/storage/not_found_error_test.go create mode 100644 test/integration/testAllowMissingFiles_test.go diff --git a/ChangeLog.md b/ChangeLog.md index 5591cd447..87c6c4193 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -8,6 +8,7 @@ NEW FEATURES - `delete local|remote ` and `POST /backup/delete/{where}/{name}` now refuse to delete a backup which other backups require via `required_backup` and report the dependent backup names, instead of silently breaking the incremental backups chain (the breakage surfaced only later, when a descendant was downloaded or restored, and for object disks the descendant `required` parts blobs were deleted together with the parent); pass `--force` (`force=1` for the API) to get the old behavior, or set `general.rebase_during_delete: true` (env `REBASE_DURING_DELETE`, default `false`) to rebase every dependent increment first (same as the `rebase` command) so the chain stays restorable — rebase copies the deleted backup parts into its dependents, so deletion time grows with the copied data size and a rebase failure aborts the delete. `backups_to_keep_local`/`backups_to_keep_remote` retention is not affected, fix [#1493](https://github.com/Altinity/clickhouse-backup/issues/1493) IMPROVEMENTS +- fail fast instead of burning the whole `retries_on_failure` x `retries_duration` backoff budget per file when a remote object is permanently missing (S3 `NoSuchKey`/404, GCS 404, Azure `BlobNotFound`, FTP/SFTP not-found) during `download`, `restore_remote` and other retried remote operations; add `general.allow_missing_files_on_download` (env `ALLOW_MISSING_FILES_ON_DOWNLOAD`, default `false`), `--allow-missing-files` CLI flag for `download`/`restore_remote` and the `allow_missing_files` query argument for `POST /backup/download` and `POST /backup/restore_remote` — salvage mode which skips data parts missing on remote storage with an `error`-level log and a final summary, drops them from local table metadata and lets the intact tables/parts of a partially corrupted backup be restored, metadata files are never skipped, fix [#1456](https://github.com/Altinity/clickhouse-backup/issues/1456) - add `s3.delete_batch_fallback_to_single` (env `S3_DELETE_BATCH_FALLBACK_TO_SINGLE`, default `true`) and `s3.delete_batch_min_size` (env `S3_DELETE_BATCH_MIN_SIZE`, default `0`) — when a whole `DeleteObjects` batch fails (some S3-compatible gateways such as DigitalOcean Spaces / Ceph RGW reset the response stream when the batch contains large objects, so retrying the same batch never succeeds and blocks retention and the `watch` loop), the batch is split in halves down to `delete_batch_min_size` and finally its objects are deleted one by one with `DeleteObject`, `s3.delete_concurrency` in parallel; `general.delete_batch_size` is now validated (1..1000 for `s3`), fix [#1532](https://github.com/Altinity/clickhouse-backup/issues/1532) - `download --hardlink-exists-files` no longer does per-part filesystem and ClickHouse lookups, which dominated the download time on servers holding many local backups or many parts. Two changes: the shadow directories of the local backups are now indexed once per `download` run from their table metadata, so finding a hardlink candidate for a backup carrying legacy CRC64 `checksums` costs one map lookup plus one `stat` instead of a `filepath.Glob` over `/backup/*/shadow/...` (which ran twice per part, once for the free space check and once for the download itself); and the `hash_of_all_files` lookup in `system.parts` is now read once per table in chunks of 1000 hashes instead of one `SELECT` per part. Both paths keep their previous results: an indexed candidate is still verified by the CRC64 of its `checksums.txt` before being hardlinked, a local backup which can't be indexed (broken backup, unreadable metadata) marks the index incomplete and restores the old glob, and a `system.parts` candidate which a merge removed after the snapshot was taken is detected before hardlinking and re-resolved by a single live query for that part, fix [#1457](https://github.com/Altinity/clickhouse-backup/issues/1457) diff --git a/ReadMe.md b/ReadMe.md index 9959df120..7ac3f92f5 100644 --- a/ReadMe.md +++ b/ReadMe.md @@ -132,6 +132,7 @@ general: # When `clickhouse->use_embedded_backup_restore: true`, throttling is delegated to the ClickHouse server via the `max_backup_bandwidth` query setting passed in the BACKUP/RESTORE SETTINGS clause (requires ClickHouse 25.1+); upload_max_bytes_per_second applies to BACKUP, download_max_bytes_per_second to RESTORE. On older ClickHouse versions embedded transfers are not throttled. download_max_bytes_per_second: 0 # DOWNLOAD_MAX_BYTES_PER_SECOND, 0 means no throttling upload_max_bytes_per_second: 0 # UPLOAD_MAX_BYTES_PER_SECOND, 0 means no throttling + allow_missing_files_on_download: false # ALLOW_MISSING_FILES_ON_DOWNLOAD, salvage mode for partially corrupted remote backups: `download` and `restore_remote` skip data parts whose files are missing on remote storage (404/NoSuchKey/BlobNotFound) with an `error` log and drop them from local table metadata instead of failing, metadata files are never skipped, requires `upload_by_part: true`, `--allow-missing-files` CLI argument or `allow_missing_files` API parameter overrides it per command, see https://github.com/Altinity/clickhouse-backup/issues/1456 download_disk_limit: 0 # DOWNLOAD_DISK_LIMIT, refuse `download` and `restore_remote` when usage of any local disk would exceed this percent (1..100) after download, 0 means no limit, `--disk-limit` CLI argument or `disk_limit` API parameter overrides it per command, see https://github.com/Altinity/clickhouse-backup/issues/1458 # MAX_BROKEN_PART_RATIO, maximum allowed fraction (0..1) of broken data parts (e.g. caused by S3-disk or filesystem failures) that still produces a successful but partial backup during backup creation (`create`, and the create stage of `create_remote`). # 0 (default) preserves legacy behavior where any broken part stops the backup completely. When >0 and the broken/total part ratio stays at or below this value, creation skips the broken parts, logs a warning, and the backup is marked successful. @@ -681,6 +682,7 @@ Download backup from remote storage: `curl -s localhost:7171/backup/download/", "command":"", "duration":""}`. When omitted or empty, falls back to `general.callback_url` if configured. Note: this operation is asynchronous, so the API will return once the operation has started. The response includes an `operation_id` field that can be used to track the operation status via `/backup/status?operationid=`. @@ -751,6 +753,7 @@ Download and restore data from remote backup: `curl -s localhost:7171/backup/res - Optional boolean query argument `resume` works the same as the `--resume` CLI argument (resume download for object disk data). - Optional boolean query argument `hardlink_exists_files` or `hardlink-exists-files` works the same as the `--hardlink-exists-files` CLI argument (Create hardlinks for existing files instead of downloading). - Optional integer query argument `disk_limit` or `disk-limit` works the same as the `--disk-limit` CLI argument (refuse download when usage of any local disk would exceed this percent after download, 1-100). +- Optional boolean query argument `allow_missing_files` or `allow-missing-files` works the same as the `--allow-missing-files` CLI argument (skip data parts missing on remote storage instead of failing, overrides `general.allow_missing_files_on_download` for this request). - Optional boolean query argument `streaming` works the same as the `--streaming` CLI argument (restore each table right after its download and delete its local copy, see [Streaming mode](#streaming-mode)). - Optional boolean query argument `skip_empty_tables` or `skip-empty-tables` works the same as the `--skip-empty-tables` CLI argument (skip restoring tables that have no data). - Optional boolean query argument `rebind_replica_path_if_exists` or `rebind-replica-path-if-exists` works the same as the `--rebind-replica-path-if-exists` CLI argument (overrides `clickhouse.rebind_replica_path_if_exists` for this request, rebind a restored ReplicatedMergeTree to `default_replica_path` when the original ZK path still has leftover state but our replica entry is absent). WARNING: never set during a concurrent HA multi-replica restore. @@ -999,6 +1002,7 @@ OPTIONS: --resume, --resumable Save intermediate download state and resume download if backup exists on local storage, ignored with 'remote_storage: custom' or 'use_embedded_backup_restore: true' --hardlink-exists-files Create hardlinks for existing files instead of downloading --disk-limit int Refuse download when usage of any local disk would exceed this percent (1-100) after download, overrides general->download_disk_limit, 0 means use config value, https://github.com/Altinity/clickhouse-backup/issues/1458 (default: 0) + --allow-missing-files Skip data part files which are missing on remote storage (404/NoSuchKey) with an error log and drop them from local table metadata instead of failing, salvage mode for partially corrupted backups, overrides general->allow_missing_files_on_download, https://github.com/Altinity/clickhouse-backup/issues/1456 --dry-run Show tables count and data size which would be downloaded, without downloading --help, -h show help @@ -1114,6 +1118,7 @@ OPTIONS: --restore-schema-as-attach Use DETACH/ATTACH instead of DROP/CREATE for schema restoration --hardlink-exists-files Create hardlinks for existing files instead of downloading --disk-limit int Refuse download when usage of any local disk would exceed this percent (1-100) after download, overrides general->download_disk_limit, 0 means use config value, https://github.com/Altinity/clickhouse-backup/issues/1458 (default: 0) + --allow-missing-files Skip data part files which are missing on remote storage (404/NoSuchKey) with an error log and drop them from local table metadata instead of failing, salvage mode for partially corrupted backups, overrides general->allow_missing_files_on_download, https://github.com/Altinity/clickhouse-backup/issues/1456 --skip-empty-tables Skip restoring tables that have no data (empty tables with only schema) --streaming Restore each table right after its download and delete its local copy, keeps only a small local footprint, https://github.com/Altinity/clickhouse-backup/issues/780 --rebind-replica-path-if-exists Override clickhouse.rebind_replica_path_if_exists, rebind a restored ReplicatedMergeTree to default_replica_path when the original ZK path still has leftover state but our replica entry is absent diff --git a/cmd/clickhouse-backup/main.go b/cmd/clickhouse-backup/main.go index 2683dfc47..e75d23e4a 100644 --- a/cmd/clickhouse-backup/main.go +++ b/cmd/clickhouse-backup/main.go @@ -527,6 +527,11 @@ func newRootCommand() *cli.Command { Hidden: false, Usage: "Refuse download when usage of any local disk would exceed this percent (1-100) after download, overrides general->download_disk_limit, 0 means use config value, https://github.com/Altinity/clickhouse-backup/issues/1458", }, + &cli.BoolFlag{ + Name: "allow-missing-files", + Hidden: false, + Usage: "Skip data part files which are missing on remote storage (404/NoSuchKey) with an error log and drop them from local table metadata instead of failing, salvage mode for partially corrupted backups, overrides general->allow_missing_files_on_download, https://github.com/Altinity/clickhouse-backup/issues/1456", + }, &cli.BoolFlag{ Name: "dry-run", Usage: "Show tables count and data size which would be downloaded, without downloading", @@ -828,6 +833,11 @@ func newRootCommand() *cli.Command { Hidden: false, Usage: "Refuse download when usage of any local disk would exceed this percent (1-100) after download, overrides general->download_disk_limit, 0 means use config value, https://github.com/Altinity/clickhouse-backup/issues/1458", }, + &cli.BoolFlag{ + Name: "allow-missing-files", + Hidden: false, + Usage: "Skip data part files which are missing on remote storage (404/NoSuchKey) with an error log and drop them from local table metadata instead of failing, salvage mode for partially corrupted backups, overrides general->allow_missing_files_on_download, https://github.com/Altinity/clickhouse-backup/issues/1456", + }, &cli.BoolFlag{ Name: "skip-empty-tables", Hidden: false, diff --git a/pkg/backup/backuper.go b/pkg/backup/backuper.go index a9b077240..5b30c0315 100644 --- a/pkg/backup/backuper.go +++ b/pkg/backup/backuper.go @@ -12,6 +12,7 @@ import ( "regexp" "strings" "sync" + "sync/atomic" "github.com/Altinity/clickhouse-backup/v2/pkg/common" "github.com/Altinity/clickhouse-backup/v2/pkg/metadata" @@ -62,6 +63,8 @@ type Backuper struct { // localPartIndex - read-only after build, maps parts of local backups to their shadow directories // so `download --hardlink-exists-files` doesn't glob all local backups per part, see issues/1457 localPartIndex *localPartIndex + // skippedMissingParts - data parts skipped by allow_missing_files_on_download during the current download, see issues/1456 + skippedMissingParts atomic.Uint64 } func NewBackuper(cfg *config.Config, opts ...BackuperOpt) *Backuper { @@ -87,6 +90,11 @@ func (b *Backuper) Classify(err error) retrier.Action { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { return retrier.Fail } + // a missing remote object (404/NoSuchKey/BlobNotFound) will never heal, don't burn the retry budget on it, + // see https://github.com/Altinity/clickhouse-backup/issues/1456 + if storage.IsNotFoundErr(err) { + return retrier.Fail + } log.Warn().Err(err).Msgf("Will wait near %s and retry", common.AddRandomJitter(b.cfg.General.RetriesDuration, b.cfg.General.RetriesJitter)) return retrier.Retry } diff --git a/pkg/backup/backuper_test.go b/pkg/backup/backuper_test.go index 83b8c8a3f..ae815f81c 100644 --- a/pkg/backup/backuper_test.go +++ b/pkg/backup/backuper_test.go @@ -25,7 +25,11 @@ func TestClassify(t *testing.T) { {context.Canceled, retrier.Fail}, {context.DeadlineExceeded, retrier.Fail}, {fmt.Errorf("object_disk.CopyObject: %w", context.Canceled), retrier.Fail}, + // https://github.com/Altinity/clickhouse-backup/issues/1456 + {fmt.Errorf("DownloadCompressedStream StatFile: %w", storage.NewErrNotFound("shadow/default/t/default_all_1_1_0.tar")), retrier.Fail}, + {&smithy.GenericAPIError{Code: "NoSuchKey", Message: "The specified key does not exist"}, retrier.Fail}, {errors.New("transient network error"), retrier.Retry}, + {&smithy.GenericAPIError{Code: "SlowDown", Message: "Please reduce your request rate"}, retrier.Retry}, } for _, tc := range testcases { if got := b.Classify(tc.err); got != tc.expect { diff --git a/pkg/backup/download.go b/pkg/backup/download.go index 4bc43f2b6..95ba01bc9 100644 --- a/pkg/backup/download.go +++ b/pkg/backup/download.go @@ -55,31 +55,6 @@ func (b *Backuper) resumeExistingBackup(backupName, command string) error { return nil } -func isRemoteMetadataNotFound(err error) bool { - if err == nil { - return false - } - message := strings.ToLower(err.Error()) - // Every remote storage backend phrases "object is missing" differently, so we - // match the known permanent-not-found markers across S3/GCS/Azure/FTP/SFTP/FS. - for _, marker := range []string{ - "doesn't exist", // GCS - "does not exist", // SFTP ("file does not exist"), Azure ("the specified blob does not exist") - "no such file or directory", // FTP (550), local filesystem - "key not found", - "nosuchkey", // S3 - "blobnotfound", // Azure Blob (x-ms-error-code) - "statuscode 404", // S3 SDK v2 - "statuscode: 404", - "status: 404", // Azure ("RESPONSE Status: 404") - } { - if strings.Contains(message, marker) { - return true - } - } - return false -} - func (b *Backuper) Download(backupName string, tablePattern string, partitions []string, schemaOnly, rbacOnly, configsOnly, namedCollectionsOnly, resume bool, hardlinkExistsFiles bool, backupVersion string, commandId int) error { if pidCheckErr := pidlock.CheckAndCreatePidFile(backupName, "download"); pidCheckErr != nil { return errors.Wrap(pidCheckErr, "CheckAndCreatePidFile") @@ -440,6 +415,9 @@ func (b *Backuper) downloadEpilogue(ctx context.Context, backupName string, remo "object_disk_size": utils.FormatBytes(backupMetadata.ObjectDiskSize), "version": backupVersion, }).Msg("done") + if skipped := b.skippedMissingParts.Load(); skipped > 0 { + log.Error().Msgf("%d data parts were missing on remote storage and skipped because allow_missing_files_on_download=true, local backup %s is partial", skipped, backupName) + } return nil } @@ -664,7 +642,7 @@ func (b *Backuper) downloadTableMetadata(ctx context.Context, backupName string, err := retry.RunCtx(ctx, func(ctx context.Context) error { tmReader, err := b.dst.GetFileReader(ctx, remoteMetadataFile) if err != nil { - if isRemoteMetadataNotFound(err) { + if storage.IsNotFoundErr(err) { metadataNotFound = true return nil } @@ -862,12 +840,91 @@ func (b *Backuper) downloadBackupRelatedDir(ctx context.Context, remoteBackup st return uint64(remoteFileInfo.Size()), nil } +// missingParts collects data parts whose files are missing on remote storage and were skipped +// because of allow_missing_files_on_download, so they can be dropped from the local table metadata +// after all download goroutines finish, see https://github.com/Altinity/clickhouse-backup/issues/1456 +type missingParts struct { + mu sync.Mutex + parts map[string]map[string]bool // disk -> part names + files map[string]map[string]bool // disk -> archive file names +} + +func (m *missingParts) addPart(disk, partName string) { + m.mu.Lock() + defer m.mu.Unlock() + if m.parts == nil { + m.parts = map[string]map[string]bool{} + } + if m.parts[disk] == nil { + m.parts[disk] = map[string]bool{} + } + m.parts[disk][partName] = true +} + +// addFile records an archive from table.Files, which is keyed by the original disk even when the part is rebalanced +func (m *missingParts) addFile(disk, archiveFile string) { + m.mu.Lock() + defer m.mu.Unlock() + if m.files == nil { + m.files = map[string]map[string]bool{} + } + if m.files[disk] == nil { + m.files[disk] = map[string]bool{} + } + m.files[disk][archiveFile] = true +} + +// apply removes the collected parts and archive files from table, returns true when table changed +func (m *missingParts) apply(table *metadata.TableMetadata) bool { + m.mu.Lock() + defer m.mu.Unlock() + changed := false + for disk, names := range m.parts { + kept := make([]metadata.Part, 0, len(table.Parts[disk])) + for _, part := range table.Parts[disk] { + if !names[part.Name] { + kept = append(kept, part) + } + } + if len(kept) != len(table.Parts[disk]) { + table.Parts[disk] = kept + changed = true + } + } + for disk, names := range m.files { + kept := make([]string, 0, len(table.Files[disk])) + for _, f := range table.Files[disk] { + if !names[f] { + kept = append(kept, f) + } + } + if len(kept) != len(table.Files[disk]) { + table.Files[disk] = kept + changed = true + } + } + return changed +} + +// skipMissingPart logs and records a part whose data is missing on remote storage when allow_missing_files_on_download is set, +// returns false when the error must be propagated instead +func (b *Backuper) skipMissingPart(err error, missing *missingParts, table metadata.TableMetadata, disk, partName, remoteFile string) bool { + if !b.cfg.General.AllowMissingFilesOnDownload || !storage.IsNotFoundErr(err) { + return false + } + log.Error().Err(err).Msgf("%s.%s part %s on disk %s is missing on remote storage (%s), skip it because allow_missing_files_on_download=true, backup will be partial", table.Database, table.Table, partName, disk, remoteFile) + missing.addPart(disk, partName) + b.skippedMissingParts.Add(1) + return true +} + func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata.BackupMetadata, table metadata.TableMetadata, disks []clickhouse.Disk, hardlinkExistsFiles bool, manifest *storage.ManifestReader) (uint64, error) { dbAndTableDir := path.Join(common.TablePathEncode(table.Database), common.TablePathEncode(table.Table)) dataGroup, dataCtx := errgroup.WithContext(ctx) dataGroup.SetLimit(int(b.cfg.General.DownloadConcurrency)) downloadedSize := uint64(0) var isRebalancedAfterHardLinks atomic.Bool + missing := &missingParts{} // one system.parts read per table replaces one per part, built before the part goroutines start // and read-only afterwards, https://github.com/Altinity/clickhouse-backup/issues/1457 @@ -965,6 +1022,18 @@ func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata. return nil }) if err != nil { + if b.cfg.General.AllowMissingFilesOnDownload && storage.IsNotFoundErr(err) { + // with upload_by_part=true each archive holds exactly one part named _., + // otherwise the archive is a size-based bundle of many parts and can't be skipped one by one + partName := strings.TrimPrefix(strings.TrimSuffix(archiveFile, "."+config.ArchiveExtensions[remoteBackup.DataFormat]), disk+"_") + for _, part := range capturedParts { + if part.Name == partName && b.skipMissingPart(err, missing, table, capturedDisk, partName, tableRemoteFile) { + missing.addFile(disk, archiveFile) + return nil + } + } + return errors.Wrapf(err, "%s is missing on remote storage and contains several parts (upload_by_part=false), can't skip it even with allow_missing_files_on_download=true", tableRemoteFile) + } return errors.Wrap(err, "DownloadCompressedStream") } atomic.AddUint64(&downloadedSize, uint64(downloadedBytes)) @@ -1063,6 +1132,9 @@ func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata. if len(manifestFiles) > 0 { pathSize, downloadErr := b.dst.DownloadPathWithManifest(dataCtx, partRemotePath, partLocalPath, manifestFiles, b.cfg.General.RetriesOnFailure, b.cfg.General.RetriesDuration, b.cfg.General.RetriesJitter, b, b.cfg.General.DownloadMaxBytesPerSecond) if downloadErr != nil { + if b.skipMissingPart(downloadErr, missing, table, capturedDisk, capturedPart.Name, partRemotePath) { + return os.RemoveAll(partLocalPath) + } return errors.WithMessage(downloadErr, "DownloadPathWithManifest") } atomic.AddUint64(&downloadedSize, uint64(pathSize)) @@ -1078,6 +1150,9 @@ func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata. // Fall back to Walk (ListObjectsV2) when no manifest is available pathSize, downloadErr := b.dst.DownloadPath(dataCtx, partRemotePath, partLocalPath, b.cfg.General.RetriesOnFailure, b.cfg.General.RetriesDuration, b.cfg.General.RetriesJitter, b, b.cfg.General.DownloadMaxBytesPerSecond) if downloadErr != nil { + if b.skipMissingPart(downloadErr, missing, table, capturedDisk, capturedPart.Name, partRemotePath) { + return os.RemoveAll(partLocalPath) + } return errors.Wrap(downloadErr, "DownloadPath") } atomic.AddUint64(&downloadedSize, uint64(pathSize)) @@ -1095,7 +1170,7 @@ func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata. if err := dataGroup.Wait(); err != nil { return 0, errors.Wrap(err, "one of downloadTableData go-routine return error") } - if isRebalancedAfterHardLinks.Load() { + if missing.apply(&table) || isRebalancedAfterHardLinks.Load() { if _, saveErr := table.Save(table.LocalFile, false); saveErr != nil { return 0, errors.Wrap(saveErr, "save rebalanced table after hardlinks") } @@ -1697,6 +1772,7 @@ func (b *Backuper) downloadDiffParts(ctx context.Context, remoteBackup metadata. downloadedDiffParts := uint32(0) downloadDiffGroup, downloadDiffCtx := errgroup.WithContext(ctx) downloadDiffGroup.SetLimit(int(b.cfg.General.DownloadConcurrency)) + missing := &missingParts{} diffRemoteFilesCache := map[string]*sync.Mutex{} diffRemoteFilesLock := &sync.Mutex{} isRebalancedAfterHardLinks := false @@ -1823,6 +1899,9 @@ func (b *Backuper) downloadDiffParts(ctx context.Context, remoteBackup metadata. for tableRemoteFile, tableLocalDir := range tableRemoteFiles { fileDiffBytes, downloadErr := b.downloadDiffRemoteFile(downloadDiffCtx, diffRemoteFilesLock, diffRemoteFilesCache, tableRemoteFile, tableLocalDir) if downloadErr != nil { + if b.skipMissingPart(downloadErr, missing, table, capturedDisk, partForDownload.Name, tableRemoteFile) { + return nil + } return errors.Wrap(downloadErr, "downloadDiffRemoteFile") } downloadedPartPath := path.Join(tableLocalDir, partForDownload.Name) @@ -1872,7 +1951,7 @@ func (b *Backuper) downloadDiffParts(ctx context.Context, remoteBackup metadata. if err := downloadDiffGroup.Wait(); err != nil { return 0, errors.Wrap(err, "one of downloadDiffParts go-routine return error") } - if isRebalancedAfterHardLinks { + if missing.apply(&table) || isRebalancedAfterHardLinks { if _, saveErr := table.Save(table.LocalFile, false); saveErr != nil { return 0, errors.Wrap(saveErr, "save rebalanced table after hardlinks in downloadDiffParts") } diff --git a/pkg/backup/download_test.go b/pkg/backup/download_test.go index 9ebfe8c27..8ba6ef6d1 100644 --- a/pkg/backup/download_test.go +++ b/pkg/backup/download_test.go @@ -2,7 +2,6 @@ package backup import ( "context" - "errors" "os" "path" "regexp" @@ -97,26 +96,6 @@ var remoteBackup = storage.Backup{ UploadDate: time.Now(), } -func TestIsRemoteMetadataNotFound(t *testing.T) { - notFoundMessages := []string{ - "object doesn't exist", - "key not found: metadata/default/test.json", - "NoSuchKey: The specified key does not exist", - "operation error S3: GetObject, https response error StatusCode: 404", - "StatusCode 404", - // real backend phrasings observed in test/integration TestMetadataNotFound* - "550 /backup/metadata/default/test.json: No such file or directory", // FTP - "file does not exist", // SFTP - "AzureBlob GetFileReaderAbsolute Download: RESPONSE ERROR (ServiceCode=BlobNotFound) RESPONSE Status: 404 The specified blob does not exist.", // Azure - } - for _, msg := range notFoundMessages { - assert.True(t, isRemoteMetadataNotFound(errors.New(msg)), msg) - } - - assert.False(t, isRemoteMetadataNotFound(nil)) - assert.False(t, isRemoteMetadataNotFound(errors.New("temporary network timeout"))) -} - func TestResumeExistingBackupMissingStateFileReturnsError(t *testing.T) { backupName := "test_resume_crash" defaultDataPath := t.TempDir() diff --git a/pkg/config/config.go b/pkg/config/config.go index 522322772..d57352112 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -100,6 +100,11 @@ type GeneralConfig struct { // DownloadDiskLimit - refuse `download` and `restore_remote` when usage of any local disk would exceed this percent (1..100) after download, // 0 (default) disables the check, `--disk-limit` CLI argument overrides it per command, see https://github.com/Altinity/clickhouse-backup/issues/1458 DownloadDiskLimit int `yaml:"download_disk_limit" envconfig:"DOWNLOAD_DISK_LIMIT"` + // AllowMissingFilesOnDownload - salvage mode for partially corrupted remote backups: when a data part file + // is missing on remote storage (404/NoSuchKey/BlobNotFound) `download` and `restore_remote` skip the part with + // an error-level log and drop it from the local table metadata instead of failing, metadata files are never skipped, + // `--allow-missing-files` CLI argument overrides it per command, see https://github.com/Altinity/clickhouse-backup/issues/1456 + AllowMissingFilesOnDownload bool `yaml:"allow_missing_files_on_download" envconfig:"ALLOW_MISSING_FILES_ON_DOWNLOAD"` // MaxBrokenPartRatio - maximum allowed fraction (0..1) of broken data parts that still produces a // successful but partial backup during backup creation (`create`, and the create stage of // `create_remote`). 0 (default) preserves legacy behavior where any broken part aborts the whole @@ -918,6 +923,7 @@ func DefaultConfig() *Config { CompressionUseMultiThread: true, MaxBrokenPartRatio: 0, DownloadDiskLimit: 0, + AllowMissingFilesOnDownload: false, }, ClickHouse: ClickHouseConfig{ Username: "default", @@ -1057,6 +1063,10 @@ func GetConfigFromCli(ctx *cli.Command) *Config { if ctx.Bool("rebind-replica-path-if-exists") { cfg.ClickHouse.RebindReplicaPathIfExists = true } + // `download`/`restore_remote` expose --allow-missing-files, same only-override-when-true semantics, see issues/1456 + if ctx.Bool("allow-missing-files") { + cfg.General.AllowMissingFilesOnDownload = true + } return cfg } diff --git a/pkg/server/server.go b/pkg/server/server.go index 04ea1d41c..3db6992f1 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -2347,6 +2347,11 @@ func (api *APIServer) httpRestoreRemoteHandler(w http.ResponseWriter, r *http.Re hardlinkExistsFiles = true fullCommand += " --hardlink-exists-files" } + // https://github.com/Altinity/clickhouse-backup/issues/1456 + if _, exist := api.getQueryParameter(query, "allow_missing_files"); exist { + cfg.General.AllowMissingFilesOnDownload = true + fullCommand += " --allow-missing-files" + } // https://github.com/Altinity/clickhouse-backup/issues/1458 diskLimit := 0 if v, exist := api.getQueryParameter(query, "disk_limit"); exist { @@ -2518,6 +2523,11 @@ func (api *APIServer) httpDownloadHandler(w http.ResponseWriter, r *http.Request hardlinkExistsFiles = true fullCommand += " --hardlink-exists-files" } + // https://github.com/Altinity/clickhouse-backup/issues/1456 + if _, exist := api.getQueryParameter(query, "allow_missing_files"); exist { + cfg.General.AllowMissingFilesOnDownload = true + fullCommand += " --allow-missing-files" + } // https://github.com/Altinity/clickhouse-backup/issues/1458 diskLimit := 0 if v, exist := api.getQueryParameter(query, "disk_limit"); exist { diff --git a/pkg/storage/not_found_error.go b/pkg/storage/not_found_error.go new file mode 100644 index 000000000..5cf9caacd --- /dev/null +++ b/pkg/storage/not_found_error.go @@ -0,0 +1,65 @@ +package storage + +import ( + "errors" + "io/fs" + "net/http" + "strings" + + "cloud.google.com/go/storage" + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror" + "github.com/aws/smithy-go" + smithyhttp "github.com/aws/smithy-go/transport/http" + "google.golang.org/api/googleapi" +) + +// IsNotFoundErr reports whether err means the remote object is permanently missing +// (S3 NoSuchKey/404, GCS ErrObjectNotExist/404, Azure BlobNotFound/404, FTP 550, SFTP/local fs.ErrNotExist), +// so retrying can never succeed, see https://github.com/Altinity/clickhouse-backup/issues/1456 +func IsNotFoundErr(err error) bool { + if err == nil { + return false + } + if errors.Is(err, ErrNotFound) || errors.Is(err, fs.ErrNotExist) || errors.Is(err, storage.ErrObjectNotExist) { + return true + } + var apiErr smithy.APIError + if errors.As(err, &apiErr) { + switch apiErr.ErrorCode() { + case "NoSuchKey", "NotFound": + return true + } + } + var httpErr *smithyhttp.ResponseError + if errors.As(err, &httpErr) && httpErr.HTTPStatusCode() == http.StatusNotFound { + return true + } + var gcpErr *googleapi.Error + if errors.As(err, &gcpErr) && gcpErr.Code == http.StatusNotFound { + return true + } + var azErr *azcore.ResponseError + if errors.As(err, &azErr) && (azErr.StatusCode == http.StatusNotFound || azErr.ErrorCode == string(bloberror.BlobNotFound)) { + return true + } + // every backend phrases "object is missing" differently and some wrap it as a plain string, + // so fall back to the known permanent-not-found markers across S3/GCS/Azure/FTP/SFTP/FS + message := strings.ToLower(err.Error()) + for _, marker := range []string{ + "doesn't exist", // GCS + "does not exist", // SFTP ("file does not exist"), Azure ("the specified blob does not exist") + "no such file or directory", // FTP (550), local filesystem + "key not found", + "nosuchkey", // S3 + "blobnotfound", // Azure Blob (x-ms-error-code) + "statuscode 404", // S3 SDK v2 + "statuscode: 404", + "status: 404", // Azure ("RESPONSE Status: 404") + } { + if strings.Contains(message, marker) { + return true + } + } + return false +} diff --git a/pkg/storage/not_found_error_test.go b/pkg/storage/not_found_error_test.go new file mode 100644 index 000000000..967937d54 --- /dev/null +++ b/pkg/storage/not_found_error_test.go @@ -0,0 +1,52 @@ +package storage + +import ( + "errors" + "io/fs" + "net/http" + "testing" + + gcs "cloud.google.com/go/storage" + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror" + "github.com/aws/smithy-go" + smithyhttp "github.com/aws/smithy-go/transport/http" + pkgerrors "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "google.golang.org/api/googleapi" +) + +func TestIsNotFoundErr(t *testing.T) { + notFoundMessages := []string{ + "object doesn't exist", + "key not found: metadata/default/test.json", + "NoSuchKey: The specified key does not exist", + "operation error S3: GetObject, https response error StatusCode: 404", + "StatusCode 404", + // real backend phrasings observed in test/integration TestMetadataNotFound* + "550 /backup/metadata/default/test.json: No such file or directory", // FTP + "file does not exist", // SFTP + "AzureBlob GetFileReaderAbsolute Download: RESPONSE ERROR (ServiceCode=BlobNotFound) RESPONSE Status: 404 The specified blob does not exist.", // Azure + } + for _, msg := range notFoundMessages { + assert.True(t, IsNotFoundErr(errors.New(msg)), msg) + } + + // typed errors from SDKs and wrapped sentinels + assert.True(t, IsNotFoundErr(NewErrNotFound("metadata/default/test.json"))) + assert.True(t, IsNotFoundErr(pkgerrors.Wrap(NewErrNotFound("k"), "DownloadCompressedStream StatFile"))) + assert.True(t, IsNotFoundErr(pkgerrors.Wrap(fs.ErrNotExist, "sftp"))) + assert.True(t, IsNotFoundErr(gcs.ErrObjectNotExist)) + assert.True(t, IsNotFoundErr(&smithy.GenericAPIError{Code: "NoSuchKey", Message: "x"})) + assert.True(t, IsNotFoundErr(&smithyhttp.ResponseError{Response: &smithyhttp.Response{Response: &http.Response{StatusCode: 404}}, Err: errors.New("x")})) + assert.True(t, IsNotFoundErr(&googleapi.Error{Code: 404})) + assert.True(t, IsNotFoundErr(&azcore.ResponseError{StatusCode: 404})) + assert.True(t, IsNotFoundErr(&azcore.ResponseError{ErrorCode: string(bloberror.BlobNotFound)})) + + assert.False(t, IsNotFoundErr(nil)) + assert.False(t, IsNotFoundErr(errors.New("temporary network timeout"))) + assert.False(t, IsNotFoundErr(&smithy.GenericAPIError{Code: "SlowDown"})) + assert.False(t, IsNotFoundErr(&smithyhttp.ResponseError{Response: &smithyhttp.Response{Response: &http.Response{StatusCode: 503}}, Err: errors.New("x")})) + assert.False(t, IsNotFoundErr(&googleapi.Error{Code: 503})) + assert.False(t, IsNotFoundErr(&azcore.ResponseError{StatusCode: 500})) +} diff --git a/test/integration/testAllowMissingFiles_test.go b/test/integration/testAllowMissingFiles_test.go new file mode 100644 index 000000000..256319479 --- /dev/null +++ b/test/integration/testAllowMissingFiles_test.go @@ -0,0 +1,114 @@ +//go:build integration + +package main + +import ( + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/rs/zerolog/log" +) + +// TestAllowMissingFilesOnDownload reproduces https://github.com/Altinity/clickhouse-backup/issues/1456: +// - upload a backup with two parts (upload_by_part=true, one archive per part), +// - delete one part archive on remote storage (MinIO via `mc`), +// - `download` without the flag must fail fast with a not-found error instead of burning the retry backoff, +// - `download --allow-missing-files` must succeed, skip the missing part with an error log, +// drop it from the local table metadata, and `restore` must bring back the surviving part only. +func TestAllowMissingFilesOnDownload(t *testing.T) { + chVer := strings.ReplaceAll(os.Getenv("CLICKHOUSE_VERSION"), ".", "_") + env, r := NewTestEnvironment(t) + env.connectWithWait(t, r, 0*time.Second, 1*time.Second, 1*time.Minute) + defer env.Cleanup(t, r) + + r.NoError(env.DockerCP("configs/config-s3.yml", "clickhouse-backup:/etc/clickhouse-backup/config.yml")) + cfgPath, _ := env.resolveConfigPaths(r, "config-s3.yml") + // MinIO only sees objects that went through its S3 API, so use `mc`; config path is relative to bucket + root := "local/clickhouse/" + cfgPath + const mcAliasCmd = "mc alias set local https://localhost:9000 access_key it_is_my_super_secret_key >/dev/null 2>&1" + + tableShort := "test_allow_missing_files" + tableName := "default." + tableShort + backupName := fmt.Sprintf("allow_missing_files_%s_%d", chVer, time.Now().UnixNano()) + // two partitions -> two parts -> two archives, background merges never cross partitions + env.queryWithNoError(t, r, fmt.Sprintf("CREATE TABLE IF NOT EXISTS %s(id UInt64, p UInt8) ENGINE=MergeTree() PARTITION BY p ORDER BY id", tableName)) + env.queryWithNoError(t, r, fmt.Sprintf("INSERT INTO %s SELECT number, 1 FROM numbers(100)", tableName)) + env.queryWithNoError(t, r, fmt.Sprintf("INSERT INTO %s SELECT number, 2 FROM numbers(50)", tableName)) + t.Cleanup(func() { + dropQ := "DROP TABLE IF EXISTS " + tableName + if compareVersion(os.Getenv("CLICKHOUSE_VERSION"), "20.3") > 0 { + dropQ += " NO DELAY" + } + if _, err := env.DockerExecOut("clickhouse", "clickhouse", "client", "-q", dropQ); err != nil { + log.Warn().Err(err).Str("table", tableName).Msg("t.Cleanup: failed to drop table") + } + }) + defer func() { + for _, cmd := range [][]string{ + {"clickhouse-backup", "delete", "local", backupName}, + {"clickhouse-backup", "delete", "remote", backupName}, + } { + if out, err := env.DockerExecOut("clickhouse-backup", cmd...); err != nil { + log.Warn().Err(err).Str("cmd", strings.Join(cmd, " ")).Msgf("allowMissingFiles teardown: %s", out) + } + } + }() + + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "create", "--tables", tableName, backupName) + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "upload", backupName) + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "delete", "local", backupName) + + // partition 2 lives in part 2_*; delete exactly that archive on remote storage + shadowDir := fmt.Sprintf("%s/%s/shadow/default/%s/", root, backupName, tableShort) + lsOut, err := env.DockerExecOut("minio", "bash", "-c", fmt.Sprintf("%s && mc ls %s", mcAliasCmd, shadowDir)) + r.NoError(err, "mc ls %s: %s", shadowDir, lsOut) + // `mc ls` prints "[date] [time] [size] [name]" per object, the archive name is the last field + archives := make([]string, 0, 2) + missingArchive := "" + for _, line := range strings.Split(strings.TrimSpace(lsOut), "\n") { + fields := strings.Fields(line) + if len(fields) == 0 { + continue + } + f := fields[len(fields)-1] + archives = append(archives, f) + if strings.HasPrefix(f, "default_2_") { + missingArchive = f + } + } + r.NotEmpty(missingArchive, "expected an archive for partition 2 in %s, got: %s", shadowDir, lsOut) + r.Len(archives, 2, "expected exactly two part archives, got: %s", lsOut) + env.DockerExecNoError(r, "minio", "bash", "-c", fmt.Sprintf("%s && mc rm %s%s", mcAliasCmd, shadowDir, missingArchive)) + + log.Debug().Msg("download without --allow-missing-files must fail fast (no retry backoff)") + start := time.Now() + out, err := env.DockerExecOut("clickhouse-backup", "clickhouse-backup", "download", backupName) + elapsed := time.Since(start) + log.Debug().Msg(out) + r.Error(err, "download must fail when a part archive is missing on remote storage, output: %s", out) + r.Contains(strings.ToLower(out), "not found", "download must report the missing archive as not-found, output: %s", out) + // "Will wait near Ns and retry" (pkg/backup/backuper.go) is logged on every retry attempt + r.NotContains(out, "and retry", "download must not retry on a permanent 404, output: %s", out) + r.Less(elapsed, 20*time.Second, "download must not burn the retry backoff on a permanent 404, took %s, output: %s", elapsed, out) + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "delete", "local", backupName) + + log.Debug().Msg("download --allow-missing-files must skip the missing part and succeed") + out, err = env.DockerExecOut("clickhouse-backup", "clickhouse-backup", "download", "--allow-missing-files", backupName) + log.Debug().Msg(out) + r.NoError(err, "download --allow-missing-files must succeed, output: %s", out) + r.Contains(out, "skip it because allow_missing_files_on_download=true", "missing part must be reported with an error log, output: %s", out) + r.Contains(out, "1 data parts were missing on remote storage and skipped", "summary must count the skipped part, output: %s", out) + r.NotContains(out, "and retry", "download must not retry on a permanent 404, output: %s", out) + + localMeta, err := env.DockerExecOut("clickhouse-backup", "cat", fmt.Sprintf("/var/lib/clickhouse/backup/%s/metadata/default/%s.json", backupName, tableShort)) + r.NoError(err, "read local table metadata: %s", localMeta) + r.NotContains(localMeta, missingArchive, "missing archive must be dropped from table metadata files: %s", localMeta) + r.NotContains(localMeta, `"name": "2_`, "missing part must be dropped from table metadata parts: %s", localMeta) + r.Contains(localMeta, `"name": "1_`, "surviving part must stay in table metadata: %s", localMeta) + + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "restore", "--rm", backupName) + env.checkCount(r, 1, 100, fmt.Sprintf("SELECT count() FROM %s SETTINGS empty_result_for_aggregation_by_empty_set=0", tableName)) +} diff --git a/test/integration/testMetadataNotFound_test.go b/test/integration/testMetadataNotFound_test.go index 748acacd7..3c0ca9d79 100644 --- a/test/integration/testMetadataNotFound_test.go +++ b/test/integration/testMetadataNotFound_test.go @@ -159,7 +159,7 @@ func runMetadataNotFoundScenario(t *testing.T, tc metadataNotFoundCase) { r.Error(err, "download must fail when remote table metadata is missing, output: %s", out) // The fail-fast path emits this distinctive message; the raw backend errors // (e.g. "no such file or directory", "BlobNotFound") do not contain it, so a - // match proves isRemoteMetadataNotFound classified the 404 correctly. + // match proves storage.IsNotFoundErr classified the 404 correctly. r.Contains(strings.ToLower(out), "not found on remote storage", "download must report the missing metadata as not-found, output: %s", out) // "Will wait near Ns and retry" (pkg/backup/backuper.go) is logged on every // retry attempt; its absence proves the permanent 404 broke out immediately. diff --git a/test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot b/test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot index d732345dc..3ca593493 100644 --- a/test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot +++ b/test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot @@ -1,4 +1,4 @@ -default_config = r"""'[\'general:\', \' remote_storage: none\', \' backups_to_keep_local: 0\', \' backups_to_keep_remote: 0\', \' log_level: info\', \' disable_environment_override: false\', \' allow_empty_backups: false\', \' rebase_before_remove_old_remote: false\', \' rebase_during_delete: false\', \' download_disk_limit: 0\', \' pipe_buffer_size: 131072\', \' download_copy_buffer_size: 0\', \' compression_use_multi_thread: true\', \' compression_threads: 0\', \' compression_buffer_size: 0\', \' allow_object_disk_streaming: false\', \' use_resumable_state: true\', \' restore_schema_on_cluster: ""\', \' upload_by_part: true\', \' download_by_part: true\', \' restore_database_mapping: {}\', \' restore_table_mapping: {}\', \' retries_on_failure: 3\', \' retries_pause: 5s\', \' retries_jitter: 0\', \' watch_interval: 1h\', \' full_interval: 24h\', \' watch_backup_name_template: shard{shard}-{type}-{time:20060102150405}\', \' callback_url: ""\', \' callback_timeout: 5s\', \' watch_schedules: []\', \' sharded_operation_mode: ""\', \' cpu_nice_priority: 15\', \' io_nice_priority: idle\', \' rbac_backup_always: true\', \' rbac_conflict_resolution: recreate\', \' config_backup_always: false\', \' named_collections_backup_always: false\', \' delete_batch_size: 1000\', \' status_history_size: 1000\', \' retriesduration: 5s\', \' watchduration: 1h0m0s\', \' fullduration: 24h0m0s\', \' callbacktimeoutduration: 5s\', \'clickhouse:\', \' username: default\', \' password: ""\', \' host: localhost\', \' port: 9000\', \' disk_mapping: {}\', \' skip_tables:\', \' - system.*\', \' - INFORMATION_SCHEMA.*\', \' - information_schema.*\', \' - _temporary_and_external_tables.*\', \' skip_table_engines: []\', \' skip_disks: []\', \' skip_disk_types: []\', \' timeout: 30m\', \' freeze_by_part: false\', \' freeze_by_part_where: ""\', \' use_embedded_backup_restore: false\', \' use_embedded_backup_restore_cluster: ""\', \' embedded_backup_disk: ""\', \' backup_mutations: true\', \' restore_as_attach: false\', \' restore_distributed_cluster: ""\', \' check_parts_columns: true\', \' parts_columns_batch_size: 25\', \' secure: false\', \' skip_verify: false\', \' sync_replicated_tables: false\', \' log_sql_queries: true\', \' config_dir: /etc/clickhouse-server/\', \' restart_command: exec:systemctl restart clickhouse-server\', \' ignore_not_exists_error_during_freeze: true\', \' check_replicas_before_attach: true\', \' default_replica_path: /clickhouse/tables/{cluster}/{shard}/{database}/{table}\', " default_replica_name: \'{replica}\'", \' rebind_replica_path_if_exists: false\', \' tls_key: ""\', \' tls_cert: ""\', \' tls_ca: ""\', \' debug: false\', \' force_rebalance: false\', \'s3:\', \' access_key: ""\', \' secret_key: ""\', \' bucket: ""\', \' endpoint: ""\', \' region: us-east-1\', \' acl: private\', \' assume_role_arn: ""\', \' force_path_style: false\', \' path: ""\', \' object_disk_path: ""\', \' disable_ssl: false\', \' compression_level: 1\', \' compression_format: tar\', \' sse: ""\', \' sse_kms_key_id: ""\', \' sse_customer_algorithm: ""\', \' sse_customer_key: ""\', \' sse_customer_key_md5: ""\', \' sse_kms_encryption_context: ""\', \' disable_cert_verification: false\', \' use_custom_storage_class: false\', \' storage_class: STANDARD\', \' custom_storage_class_map: {}\', \' allow_multipart_download: false\', \' object_labels: {}\', \' request_payer: ""\', \' check_sum_algorithm: ""\', \' request_content_md5: false\', \' retry_mode: standard\', \' chunk_size: 5242880\', \' delete_batch_min_size: 0\', \' delete_batch_fallback_to_single: true\', \' debug: false\', \' http_write_buffer_size: 0\', \' http_read_buffer_size: 0\', \' http_idle_conn_timeout: ""\', \' http2_send_ping_timeout: 30s\', \' http2_ping_timeout: 15s\', \' http2_write_byte_timeout: 60s\', \'gcs:\', \' credentials_file: ""\', \' credentials_json: ""\', \' credentials_json_encoded: ""\', \' sa_email: ""\', \' embedded_access_key: ""\', \' embedded_secret_key: ""\', \' skip_credentials: false\', \' bucket: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_level: 1\', \' compression_format: tar\', \' debug: false\', \' force_http: false\', \' disable_http2: false\', \' endpoint: ""\', \' storage_class: STANDARD\', \' object_labels: {}\', \' custom_storage_class_map: {}\', \' chunk_size: 16777216\', \' encryption_key: ""\', \' upload_buffer_size: 131072\', \' allow_multipart_upload: false\', \' multipart_upload_min_size: 1073741824\', \' allow_multipart_download: false\', \'cos:\', \' url: ""\', \' timeout: 2m\', \' secret_id: ""\', \' secret_key: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' allow_multipart_download: false\', \' debug: false\', \'api:\', \' listen: localhost:7171\', \' enable_metrics: true\', \' enable_pprof: false\', \' username: ""\', \' password: ""\', \' secure: false\', \' certificate_file: ""\', \' private_key_file: ""\', \' ca_cert_file: ""\', \' ca_key_file: ""\', \' create_integration_tables: false\', \' integration_tables_host: ""\', \' allow_parallel: false\', \' complete_resumable_after_restart: true\', \' complete_resumable_after_restart_commands:\', \' - upload\', \' - download\', \' watch_is_main_process: false\', \' backup_actions_skip_commands: []\', \' cancel_operation_timeout: 1800s\', \'ftp:\', \' address: ""\', \' timeout: 2m\', \' username: ""\', \' password: ""\', \' tls: false\', \' skip_tls_verify: false\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' debug: false\', \'sftp:\', \' address: ""\', \' port: 22\', \' username: ""\', \' password: ""\', \' key: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' debug: false\', \'azblob:\', \' endpoint_schema: https\', \' endpoint_suffix: core.windows.net\', \' account_name: ""\', \' account_key: ""\', \' sas: ""\', \' use_managed_identity: false\', \' container: ""\', \' assume_container_exists: false\', \' path: ""\', \' object_disk_path: ""\', \' compression_level: 1\', \' compression_format: tar\', \' sse_key: ""\', \' buffer_count: 3\', \' timeout: 4h\', \' debug: false\', \'custom:\', \' upload_command: ""\', \' download_command: ""\', \' list_command: ""\', \' delete_command: ""\', \' command_timeout: 4h\', \' commandtimeoutduration: 4h0m0s\']'""" +default_config = r"""'[\'general:\', \' remote_storage: none\', \' backups_to_keep_local: 0\', \' backups_to_keep_remote: 0\', \' log_level: info\', \' disable_environment_override: false\', \' allow_empty_backups: false\', \' rebase_before_remove_old_remote: false\', \' rebase_during_delete: false\', \' download_disk_limit: 0\', \' allow_missing_files_on_download: false\', \' pipe_buffer_size: 131072\', \' download_copy_buffer_size: 0\', \' compression_use_multi_thread: true\', \' compression_threads: 0\', \' compression_buffer_size: 0\', \' allow_object_disk_streaming: false\', \' use_resumable_state: true\', \' restore_schema_on_cluster: ""\', \' upload_by_part: true\', \' download_by_part: true\', \' restore_database_mapping: {}\', \' restore_table_mapping: {}\', \' retries_on_failure: 3\', \' retries_pause: 5s\', \' retries_jitter: 0\', \' watch_interval: 1h\', \' full_interval: 24h\', \' watch_backup_name_template: shard{shard}-{type}-{time:20060102150405}\', \' callback_url: ""\', \' callback_timeout: 5s\', \' watch_schedules: []\', \' sharded_operation_mode: ""\', \' cpu_nice_priority: 15\', \' io_nice_priority: idle\', \' rbac_backup_always: true\', \' rbac_conflict_resolution: recreate\', \' config_backup_always: false\', \' named_collections_backup_always: false\', \' delete_batch_size: 1000\', \' status_history_size: 1000\', \' retriesduration: 5s\', \' watchduration: 1h0m0s\', \' fullduration: 24h0m0s\', \' callbacktimeoutduration: 5s\', \'clickhouse:\', \' username: default\', \' password: ""\', \' host: localhost\', \' port: 9000\', \' disk_mapping: {}\', \' skip_tables:\', \' - system.*\', \' - INFORMATION_SCHEMA.*\', \' - information_schema.*\', \' - _temporary_and_external_tables.*\', \' skip_table_engines: []\', \' skip_disks: []\', \' skip_disk_types: []\', \' timeout: 30m\', \' freeze_by_part: false\', \' freeze_by_part_where: ""\', \' use_embedded_backup_restore: false\', \' use_embedded_backup_restore_cluster: ""\', \' embedded_backup_disk: ""\', \' backup_mutations: true\', \' restore_as_attach: false\', \' restore_distributed_cluster: ""\', \' check_parts_columns: true\', \' parts_columns_batch_size: 25\', \' secure: false\', \' skip_verify: false\', \' sync_replicated_tables: false\', \' log_sql_queries: true\', \' config_dir: /etc/clickhouse-server/\', \' restart_command: exec:systemctl restart clickhouse-server\', \' ignore_not_exists_error_during_freeze: true\', \' check_replicas_before_attach: true\', \' default_replica_path: /clickhouse/tables/{cluster}/{shard}/{database}/{table}\', " default_replica_name: \'{replica}\'", \' rebind_replica_path_if_exists: false\', \' tls_key: ""\', \' tls_cert: ""\', \' tls_ca: ""\', \' debug: false\', \' force_rebalance: false\', \'s3:\', \' access_key: ""\', \' secret_key: ""\', \' bucket: ""\', \' endpoint: ""\', \' region: us-east-1\', \' acl: private\', \' assume_role_arn: ""\', \' force_path_style: false\', \' path: ""\', \' object_disk_path: ""\', \' disable_ssl: false\', \' compression_level: 1\', \' compression_format: tar\', \' sse: ""\', \' sse_kms_key_id: ""\', \' sse_customer_algorithm: ""\', \' sse_customer_key: ""\', \' sse_customer_key_md5: ""\', \' sse_kms_encryption_context: ""\', \' disable_cert_verification: false\', \' use_custom_storage_class: false\', \' storage_class: STANDARD\', \' custom_storage_class_map: {}\', \' allow_multipart_download: false\', \' object_labels: {}\', \' request_payer: ""\', \' check_sum_algorithm: ""\', \' request_content_md5: false\', \' retry_mode: standard\', \' chunk_size: 5242880\', \' delete_batch_min_size: 0\', \' delete_batch_fallback_to_single: true\', \' debug: false\', \' http_write_buffer_size: 0\', \' http_read_buffer_size: 0\', \' http_idle_conn_timeout: ""\', \' http2_send_ping_timeout: 30s\', \' http2_ping_timeout: 15s\', \' http2_write_byte_timeout: 60s\', \'gcs:\', \' credentials_file: ""\', \' credentials_json: ""\', \' credentials_json_encoded: ""\', \' sa_email: ""\', \' embedded_access_key: ""\', \' embedded_secret_key: ""\', \' skip_credentials: false\', \' bucket: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_level: 1\', \' compression_format: tar\', \' debug: false\', \' force_http: false\', \' disable_http2: false\', \' endpoint: ""\', \' storage_class: STANDARD\', \' object_labels: {}\', \' custom_storage_class_map: {}\', \' chunk_size: 16777216\', \' encryption_key: ""\', \' upload_buffer_size: 131072\', \' allow_multipart_upload: false\', \' multipart_upload_min_size: 1073741824\', \' allow_multipart_download: false\', \'cos:\', \' url: ""\', \' timeout: 2m\', \' secret_id: ""\', \' secret_key: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' allow_multipart_download: false\', \' debug: false\', \'api:\', \' listen: localhost:7171\', \' enable_metrics: true\', \' enable_pprof: false\', \' username: ""\', \' password: ""\', \' secure: false\', \' certificate_file: ""\', \' private_key_file: ""\', \' ca_cert_file: ""\', \' ca_key_file: ""\', \' create_integration_tables: false\', \' integration_tables_host: ""\', \' allow_parallel: false\', \' complete_resumable_after_restart: true\', \' complete_resumable_after_restart_commands:\', \' - upload\', \' - download\', \' watch_is_main_process: false\', \' backup_actions_skip_commands: []\', \' cancel_operation_timeout: 1800s\', \'ftp:\', \' address: ""\', \' timeout: 2m\', \' username: ""\', \' password: ""\', \' tls: false\', \' skip_tls_verify: false\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' debug: false\', \'sftp:\', \' address: ""\', \' port: 22\', \' username: ""\', \' password: ""\', \' key: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' debug: false\', \'azblob:\', \' endpoint_schema: https\', \' endpoint_suffix: core.windows.net\', \' account_name: ""\', \' account_key: ""\', \' sas: ""\', \' use_managed_identity: false\', \' container: ""\', \' assume_container_exists: false\', \' path: ""\', \' object_disk_path: ""\', \' compression_level: 1\', \' compression_format: tar\', \' sse_key: ""\', \' buffer_count: 3\', \' timeout: 4h\', \' debug: false\', \'custom:\', \' upload_command: ""\', \' download_command: ""\', \' list_command: ""\', \' delete_command: ""\', \' command_timeout: 4h\', \' commandtimeoutduration: 4h0m0s\']'""" help_flag = r"""'NAME:\n clickhouse-backup - Tool for easy backup of ClickHouse with cloud supportUSAGE:\n clickhouse-backup [-t, --tables=.] DESCRIPTION:\n Run as \'root\' or \'clickhouse\' userCOMMANDS:\n tables List of tables, exclude skip_tables\n create Create new backup\n create_remote Create and upload new backup\n upload Upload backup to remote storage\n list List of backups\n download Download backup from remote storage\n rebase Copy required parts from `required_backup` chain into remote backup and remove `required_backup` dependency, so backup becomes full\n rebalance Move data parts inside local backup between disks to match current system.parts layout and storage policy, skip parts on object disks\n restore Create schema and restore data from backup\n restore_remote Download and restore\n restore_cloud Restore ClickHouse Cloud native S3 backup (Shared engines) as Atomic databases and Replicated*MergeTree tables on the current server\n delete Delete specific backup\n default-config Print default config\n print-config Print current config merged with environment variables\n clean Remove data in \'shadow\' folder from all \'path\' folders available from \'system.disks\'\n clean_remote_broken Remove all broken remote backups\n clean_local_broken Remove all broken local backups\n clean_broken_retention Remove orphan entries under remote `path` and `object_disks_path` that are not in the live backup list\n watch Run infinite loop which create full + incremental backup sequence to allow efficient backup sequences\n acvp Run ACVP wrapper protocol over stdin/stdout\n server Run API server\n help, h Shows a list of commands or help for one commandGLOBAL OPTIONS:\n --config string, -c string Config \'FILE\' name. (default: "/etc/clickhouse-backup/config.yml") [$CLICKHOUSE_BACKUP_CONFIG]\n --environment-override string, --env string [ --environment-override string, --env string ] override any environment variable via CLI parameter\n --fips-info Display FIPS build/runtime info and exit (no Go toolchain required).\n --help, -h show help\n --version, -v print the version'""" From c0c3c4d9f53faff51aae91f4f0cb6132dcb86408 Mon Sep 17 00:00:00 2001 From: slach Date: Wed, 9 Sep 2026 20:18:35 +0500 Subject: [PATCH 2/3] fix https://github.com/Altinity/clickhouse-backup/issues/1456 Fail fast on missing remote objects and add allow_missing_files_on_download Backuper.Classify retried every non-context error, so a permanently missing remote object (S3 NoSuchKey/404, GCS 404, Azure BlobNotFound, FTP/SFTP not-found) burned the whole retries_on_failure x retries_duration backoff per file. storage.IsNotFoundErr now recognises these across backends (typed SDK errors plus the string markers that used to live in isRemoteMetadataNotFound) and Classify returns Fail. Add general.allow_missing_files_on_download (ALLOW_MISSING_FILES_ON_DOWNLOAD, default false), --allow-missing-files for download/restore_remote and the allow_missing_files API query argument: a salvage mode which skips data parts missing on remote storage with an error-level log and a final summary, drops them from the local table metadata (parts + files) and lets the intact parts of a partially corrupted backup be restored. Metadata files are never skipped; upload_by_part=false bundles stay fatal because a single part can't be carved out of a shared archive. --- ChangeLog.md | 1 + ReadMe.md | 5 + cmd/clickhouse-backup/main.go | 10 ++ pkg/backup/backuper.go | 8 ++ pkg/backup/backuper_test.go | 4 + pkg/backup/download.go | 135 ++++++++++++++---- pkg/backup/download_test.go | 21 --- pkg/config/config.go | 10 ++ pkg/server/server.go | 10 ++ pkg/storage/not_found_error.go | 65 +++++++++ pkg/storage/not_found_error_test.go | 52 +++++++ .../integration/testAllowMissingFiles_test.go | 114 +++++++++++++++ test/integration/testMetadataNotFound_test.go | 2 +- .../tests/snapshots/cli.py.cli.snapshot | 2 +- 14 files changed, 388 insertions(+), 51 deletions(-) create mode 100644 pkg/storage/not_found_error.go create mode 100644 pkg/storage/not_found_error_test.go create mode 100644 test/integration/testAllowMissingFiles_test.go diff --git a/ChangeLog.md b/ChangeLog.md index 5591cd447..87c6c4193 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -8,6 +8,7 @@ NEW FEATURES - `delete local|remote ` and `POST /backup/delete/{where}/{name}` now refuse to delete a backup which other backups require via `required_backup` and report the dependent backup names, instead of silently breaking the incremental backups chain (the breakage surfaced only later, when a descendant was downloaded or restored, and for object disks the descendant `required` parts blobs were deleted together with the parent); pass `--force` (`force=1` for the API) to get the old behavior, or set `general.rebase_during_delete: true` (env `REBASE_DURING_DELETE`, default `false`) to rebase every dependent increment first (same as the `rebase` command) so the chain stays restorable — rebase copies the deleted backup parts into its dependents, so deletion time grows with the copied data size and a rebase failure aborts the delete. `backups_to_keep_local`/`backups_to_keep_remote` retention is not affected, fix [#1493](https://github.com/Altinity/clickhouse-backup/issues/1493) IMPROVEMENTS +- fail fast instead of burning the whole `retries_on_failure` x `retries_duration` backoff budget per file when a remote object is permanently missing (S3 `NoSuchKey`/404, GCS 404, Azure `BlobNotFound`, FTP/SFTP not-found) during `download`, `restore_remote` and other retried remote operations; add `general.allow_missing_files_on_download` (env `ALLOW_MISSING_FILES_ON_DOWNLOAD`, default `false`), `--allow-missing-files` CLI flag for `download`/`restore_remote` and the `allow_missing_files` query argument for `POST /backup/download` and `POST /backup/restore_remote` — salvage mode which skips data parts missing on remote storage with an `error`-level log and a final summary, drops them from local table metadata and lets the intact tables/parts of a partially corrupted backup be restored, metadata files are never skipped, fix [#1456](https://github.com/Altinity/clickhouse-backup/issues/1456) - add `s3.delete_batch_fallback_to_single` (env `S3_DELETE_BATCH_FALLBACK_TO_SINGLE`, default `true`) and `s3.delete_batch_min_size` (env `S3_DELETE_BATCH_MIN_SIZE`, default `0`) — when a whole `DeleteObjects` batch fails (some S3-compatible gateways such as DigitalOcean Spaces / Ceph RGW reset the response stream when the batch contains large objects, so retrying the same batch never succeeds and blocks retention and the `watch` loop), the batch is split in halves down to `delete_batch_min_size` and finally its objects are deleted one by one with `DeleteObject`, `s3.delete_concurrency` in parallel; `general.delete_batch_size` is now validated (1..1000 for `s3`), fix [#1532](https://github.com/Altinity/clickhouse-backup/issues/1532) - `download --hardlink-exists-files` no longer does per-part filesystem and ClickHouse lookups, which dominated the download time on servers holding many local backups or many parts. Two changes: the shadow directories of the local backups are now indexed once per `download` run from their table metadata, so finding a hardlink candidate for a backup carrying legacy CRC64 `checksums` costs one map lookup plus one `stat` instead of a `filepath.Glob` over `/backup/*/shadow/...` (which ran twice per part, once for the free space check and once for the download itself); and the `hash_of_all_files` lookup in `system.parts` is now read once per table in chunks of 1000 hashes instead of one `SELECT` per part. Both paths keep their previous results: an indexed candidate is still verified by the CRC64 of its `checksums.txt` before being hardlinked, a local backup which can't be indexed (broken backup, unreadable metadata) marks the index incomplete and restores the old glob, and a `system.parts` candidate which a merge removed after the snapshot was taken is detected before hardlinking and re-resolved by a single live query for that part, fix [#1457](https://github.com/Altinity/clickhouse-backup/issues/1457) diff --git a/ReadMe.md b/ReadMe.md index 9959df120..7ac3f92f5 100644 --- a/ReadMe.md +++ b/ReadMe.md @@ -132,6 +132,7 @@ general: # When `clickhouse->use_embedded_backup_restore: true`, throttling is delegated to the ClickHouse server via the `max_backup_bandwidth` query setting passed in the BACKUP/RESTORE SETTINGS clause (requires ClickHouse 25.1+); upload_max_bytes_per_second applies to BACKUP, download_max_bytes_per_second to RESTORE. On older ClickHouse versions embedded transfers are not throttled. download_max_bytes_per_second: 0 # DOWNLOAD_MAX_BYTES_PER_SECOND, 0 means no throttling upload_max_bytes_per_second: 0 # UPLOAD_MAX_BYTES_PER_SECOND, 0 means no throttling + allow_missing_files_on_download: false # ALLOW_MISSING_FILES_ON_DOWNLOAD, salvage mode for partially corrupted remote backups: `download` and `restore_remote` skip data parts whose files are missing on remote storage (404/NoSuchKey/BlobNotFound) with an `error` log and drop them from local table metadata instead of failing, metadata files are never skipped, requires `upload_by_part: true`, `--allow-missing-files` CLI argument or `allow_missing_files` API parameter overrides it per command, see https://github.com/Altinity/clickhouse-backup/issues/1456 download_disk_limit: 0 # DOWNLOAD_DISK_LIMIT, refuse `download` and `restore_remote` when usage of any local disk would exceed this percent (1..100) after download, 0 means no limit, `--disk-limit` CLI argument or `disk_limit` API parameter overrides it per command, see https://github.com/Altinity/clickhouse-backup/issues/1458 # MAX_BROKEN_PART_RATIO, maximum allowed fraction (0..1) of broken data parts (e.g. caused by S3-disk or filesystem failures) that still produces a successful but partial backup during backup creation (`create`, and the create stage of `create_remote`). # 0 (default) preserves legacy behavior where any broken part stops the backup completely. When >0 and the broken/total part ratio stays at or below this value, creation skips the broken parts, logs a warning, and the backup is marked successful. @@ -681,6 +682,7 @@ Download backup from remote storage: `curl -s localhost:7171/backup/download/", "command":"", "duration":""}`. When omitted or empty, falls back to `general.callback_url` if configured. Note: this operation is asynchronous, so the API will return once the operation has started. The response includes an `operation_id` field that can be used to track the operation status via `/backup/status?operationid=`. @@ -751,6 +753,7 @@ Download and restore data from remote backup: `curl -s localhost:7171/backup/res - Optional boolean query argument `resume` works the same as the `--resume` CLI argument (resume download for object disk data). - Optional boolean query argument `hardlink_exists_files` or `hardlink-exists-files` works the same as the `--hardlink-exists-files` CLI argument (Create hardlinks for existing files instead of downloading). - Optional integer query argument `disk_limit` or `disk-limit` works the same as the `--disk-limit` CLI argument (refuse download when usage of any local disk would exceed this percent after download, 1-100). +- Optional boolean query argument `allow_missing_files` or `allow-missing-files` works the same as the `--allow-missing-files` CLI argument (skip data parts missing on remote storage instead of failing, overrides `general.allow_missing_files_on_download` for this request). - Optional boolean query argument `streaming` works the same as the `--streaming` CLI argument (restore each table right after its download and delete its local copy, see [Streaming mode](#streaming-mode)). - Optional boolean query argument `skip_empty_tables` or `skip-empty-tables` works the same as the `--skip-empty-tables` CLI argument (skip restoring tables that have no data). - Optional boolean query argument `rebind_replica_path_if_exists` or `rebind-replica-path-if-exists` works the same as the `--rebind-replica-path-if-exists` CLI argument (overrides `clickhouse.rebind_replica_path_if_exists` for this request, rebind a restored ReplicatedMergeTree to `default_replica_path` when the original ZK path still has leftover state but our replica entry is absent). WARNING: never set during a concurrent HA multi-replica restore. @@ -999,6 +1002,7 @@ OPTIONS: --resume, --resumable Save intermediate download state and resume download if backup exists on local storage, ignored with 'remote_storage: custom' or 'use_embedded_backup_restore: true' --hardlink-exists-files Create hardlinks for existing files instead of downloading --disk-limit int Refuse download when usage of any local disk would exceed this percent (1-100) after download, overrides general->download_disk_limit, 0 means use config value, https://github.com/Altinity/clickhouse-backup/issues/1458 (default: 0) + --allow-missing-files Skip data part files which are missing on remote storage (404/NoSuchKey) with an error log and drop them from local table metadata instead of failing, salvage mode for partially corrupted backups, overrides general->allow_missing_files_on_download, https://github.com/Altinity/clickhouse-backup/issues/1456 --dry-run Show tables count and data size which would be downloaded, without downloading --help, -h show help @@ -1114,6 +1118,7 @@ OPTIONS: --restore-schema-as-attach Use DETACH/ATTACH instead of DROP/CREATE for schema restoration --hardlink-exists-files Create hardlinks for existing files instead of downloading --disk-limit int Refuse download when usage of any local disk would exceed this percent (1-100) after download, overrides general->download_disk_limit, 0 means use config value, https://github.com/Altinity/clickhouse-backup/issues/1458 (default: 0) + --allow-missing-files Skip data part files which are missing on remote storage (404/NoSuchKey) with an error log and drop them from local table metadata instead of failing, salvage mode for partially corrupted backups, overrides general->allow_missing_files_on_download, https://github.com/Altinity/clickhouse-backup/issues/1456 --skip-empty-tables Skip restoring tables that have no data (empty tables with only schema) --streaming Restore each table right after its download and delete its local copy, keeps only a small local footprint, https://github.com/Altinity/clickhouse-backup/issues/780 --rebind-replica-path-if-exists Override clickhouse.rebind_replica_path_if_exists, rebind a restored ReplicatedMergeTree to default_replica_path when the original ZK path still has leftover state but our replica entry is absent diff --git a/cmd/clickhouse-backup/main.go b/cmd/clickhouse-backup/main.go index 2683dfc47..e75d23e4a 100644 --- a/cmd/clickhouse-backup/main.go +++ b/cmd/clickhouse-backup/main.go @@ -527,6 +527,11 @@ func newRootCommand() *cli.Command { Hidden: false, Usage: "Refuse download when usage of any local disk would exceed this percent (1-100) after download, overrides general->download_disk_limit, 0 means use config value, https://github.com/Altinity/clickhouse-backup/issues/1458", }, + &cli.BoolFlag{ + Name: "allow-missing-files", + Hidden: false, + Usage: "Skip data part files which are missing on remote storage (404/NoSuchKey) with an error log and drop them from local table metadata instead of failing, salvage mode for partially corrupted backups, overrides general->allow_missing_files_on_download, https://github.com/Altinity/clickhouse-backup/issues/1456", + }, &cli.BoolFlag{ Name: "dry-run", Usage: "Show tables count and data size which would be downloaded, without downloading", @@ -828,6 +833,11 @@ func newRootCommand() *cli.Command { Hidden: false, Usage: "Refuse download when usage of any local disk would exceed this percent (1-100) after download, overrides general->download_disk_limit, 0 means use config value, https://github.com/Altinity/clickhouse-backup/issues/1458", }, + &cli.BoolFlag{ + Name: "allow-missing-files", + Hidden: false, + Usage: "Skip data part files which are missing on remote storage (404/NoSuchKey) with an error log and drop them from local table metadata instead of failing, salvage mode for partially corrupted backups, overrides general->allow_missing_files_on_download, https://github.com/Altinity/clickhouse-backup/issues/1456", + }, &cli.BoolFlag{ Name: "skip-empty-tables", Hidden: false, diff --git a/pkg/backup/backuper.go b/pkg/backup/backuper.go index a9b077240..5b30c0315 100644 --- a/pkg/backup/backuper.go +++ b/pkg/backup/backuper.go @@ -12,6 +12,7 @@ import ( "regexp" "strings" "sync" + "sync/atomic" "github.com/Altinity/clickhouse-backup/v2/pkg/common" "github.com/Altinity/clickhouse-backup/v2/pkg/metadata" @@ -62,6 +63,8 @@ type Backuper struct { // localPartIndex - read-only after build, maps parts of local backups to their shadow directories // so `download --hardlink-exists-files` doesn't glob all local backups per part, see issues/1457 localPartIndex *localPartIndex + // skippedMissingParts - data parts skipped by allow_missing_files_on_download during the current download, see issues/1456 + skippedMissingParts atomic.Uint64 } func NewBackuper(cfg *config.Config, opts ...BackuperOpt) *Backuper { @@ -87,6 +90,11 @@ func (b *Backuper) Classify(err error) retrier.Action { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { return retrier.Fail } + // a missing remote object (404/NoSuchKey/BlobNotFound) will never heal, don't burn the retry budget on it, + // see https://github.com/Altinity/clickhouse-backup/issues/1456 + if storage.IsNotFoundErr(err) { + return retrier.Fail + } log.Warn().Err(err).Msgf("Will wait near %s and retry", common.AddRandomJitter(b.cfg.General.RetriesDuration, b.cfg.General.RetriesJitter)) return retrier.Retry } diff --git a/pkg/backup/backuper_test.go b/pkg/backup/backuper_test.go index 83b8c8a3f..ae815f81c 100644 --- a/pkg/backup/backuper_test.go +++ b/pkg/backup/backuper_test.go @@ -25,7 +25,11 @@ func TestClassify(t *testing.T) { {context.Canceled, retrier.Fail}, {context.DeadlineExceeded, retrier.Fail}, {fmt.Errorf("object_disk.CopyObject: %w", context.Canceled), retrier.Fail}, + // https://github.com/Altinity/clickhouse-backup/issues/1456 + {fmt.Errorf("DownloadCompressedStream StatFile: %w", storage.NewErrNotFound("shadow/default/t/default_all_1_1_0.tar")), retrier.Fail}, + {&smithy.GenericAPIError{Code: "NoSuchKey", Message: "The specified key does not exist"}, retrier.Fail}, {errors.New("transient network error"), retrier.Retry}, + {&smithy.GenericAPIError{Code: "SlowDown", Message: "Please reduce your request rate"}, retrier.Retry}, } for _, tc := range testcases { if got := b.Classify(tc.err); got != tc.expect { diff --git a/pkg/backup/download.go b/pkg/backup/download.go index 4bc43f2b6..95ba01bc9 100644 --- a/pkg/backup/download.go +++ b/pkg/backup/download.go @@ -55,31 +55,6 @@ func (b *Backuper) resumeExistingBackup(backupName, command string) error { return nil } -func isRemoteMetadataNotFound(err error) bool { - if err == nil { - return false - } - message := strings.ToLower(err.Error()) - // Every remote storage backend phrases "object is missing" differently, so we - // match the known permanent-not-found markers across S3/GCS/Azure/FTP/SFTP/FS. - for _, marker := range []string{ - "doesn't exist", // GCS - "does not exist", // SFTP ("file does not exist"), Azure ("the specified blob does not exist") - "no such file or directory", // FTP (550), local filesystem - "key not found", - "nosuchkey", // S3 - "blobnotfound", // Azure Blob (x-ms-error-code) - "statuscode 404", // S3 SDK v2 - "statuscode: 404", - "status: 404", // Azure ("RESPONSE Status: 404") - } { - if strings.Contains(message, marker) { - return true - } - } - return false -} - func (b *Backuper) Download(backupName string, tablePattern string, partitions []string, schemaOnly, rbacOnly, configsOnly, namedCollectionsOnly, resume bool, hardlinkExistsFiles bool, backupVersion string, commandId int) error { if pidCheckErr := pidlock.CheckAndCreatePidFile(backupName, "download"); pidCheckErr != nil { return errors.Wrap(pidCheckErr, "CheckAndCreatePidFile") @@ -440,6 +415,9 @@ func (b *Backuper) downloadEpilogue(ctx context.Context, backupName string, remo "object_disk_size": utils.FormatBytes(backupMetadata.ObjectDiskSize), "version": backupVersion, }).Msg("done") + if skipped := b.skippedMissingParts.Load(); skipped > 0 { + log.Error().Msgf("%d data parts were missing on remote storage and skipped because allow_missing_files_on_download=true, local backup %s is partial", skipped, backupName) + } return nil } @@ -664,7 +642,7 @@ func (b *Backuper) downloadTableMetadata(ctx context.Context, backupName string, err := retry.RunCtx(ctx, func(ctx context.Context) error { tmReader, err := b.dst.GetFileReader(ctx, remoteMetadataFile) if err != nil { - if isRemoteMetadataNotFound(err) { + if storage.IsNotFoundErr(err) { metadataNotFound = true return nil } @@ -862,12 +840,91 @@ func (b *Backuper) downloadBackupRelatedDir(ctx context.Context, remoteBackup st return uint64(remoteFileInfo.Size()), nil } +// missingParts collects data parts whose files are missing on remote storage and were skipped +// because of allow_missing_files_on_download, so they can be dropped from the local table metadata +// after all download goroutines finish, see https://github.com/Altinity/clickhouse-backup/issues/1456 +type missingParts struct { + mu sync.Mutex + parts map[string]map[string]bool // disk -> part names + files map[string]map[string]bool // disk -> archive file names +} + +func (m *missingParts) addPart(disk, partName string) { + m.mu.Lock() + defer m.mu.Unlock() + if m.parts == nil { + m.parts = map[string]map[string]bool{} + } + if m.parts[disk] == nil { + m.parts[disk] = map[string]bool{} + } + m.parts[disk][partName] = true +} + +// addFile records an archive from table.Files, which is keyed by the original disk even when the part is rebalanced +func (m *missingParts) addFile(disk, archiveFile string) { + m.mu.Lock() + defer m.mu.Unlock() + if m.files == nil { + m.files = map[string]map[string]bool{} + } + if m.files[disk] == nil { + m.files[disk] = map[string]bool{} + } + m.files[disk][archiveFile] = true +} + +// apply removes the collected parts and archive files from table, returns true when table changed +func (m *missingParts) apply(table *metadata.TableMetadata) bool { + m.mu.Lock() + defer m.mu.Unlock() + changed := false + for disk, names := range m.parts { + kept := make([]metadata.Part, 0, len(table.Parts[disk])) + for _, part := range table.Parts[disk] { + if !names[part.Name] { + kept = append(kept, part) + } + } + if len(kept) != len(table.Parts[disk]) { + table.Parts[disk] = kept + changed = true + } + } + for disk, names := range m.files { + kept := make([]string, 0, len(table.Files[disk])) + for _, f := range table.Files[disk] { + if !names[f] { + kept = append(kept, f) + } + } + if len(kept) != len(table.Files[disk]) { + table.Files[disk] = kept + changed = true + } + } + return changed +} + +// skipMissingPart logs and records a part whose data is missing on remote storage when allow_missing_files_on_download is set, +// returns false when the error must be propagated instead +func (b *Backuper) skipMissingPart(err error, missing *missingParts, table metadata.TableMetadata, disk, partName, remoteFile string) bool { + if !b.cfg.General.AllowMissingFilesOnDownload || !storage.IsNotFoundErr(err) { + return false + } + log.Error().Err(err).Msgf("%s.%s part %s on disk %s is missing on remote storage (%s), skip it because allow_missing_files_on_download=true, backup will be partial", table.Database, table.Table, partName, disk, remoteFile) + missing.addPart(disk, partName) + b.skippedMissingParts.Add(1) + return true +} + func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata.BackupMetadata, table metadata.TableMetadata, disks []clickhouse.Disk, hardlinkExistsFiles bool, manifest *storage.ManifestReader) (uint64, error) { dbAndTableDir := path.Join(common.TablePathEncode(table.Database), common.TablePathEncode(table.Table)) dataGroup, dataCtx := errgroup.WithContext(ctx) dataGroup.SetLimit(int(b.cfg.General.DownloadConcurrency)) downloadedSize := uint64(0) var isRebalancedAfterHardLinks atomic.Bool + missing := &missingParts{} // one system.parts read per table replaces one per part, built before the part goroutines start // and read-only afterwards, https://github.com/Altinity/clickhouse-backup/issues/1457 @@ -965,6 +1022,18 @@ func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata. return nil }) if err != nil { + if b.cfg.General.AllowMissingFilesOnDownload && storage.IsNotFoundErr(err) { + // with upload_by_part=true each archive holds exactly one part named _., + // otherwise the archive is a size-based bundle of many parts and can't be skipped one by one + partName := strings.TrimPrefix(strings.TrimSuffix(archiveFile, "."+config.ArchiveExtensions[remoteBackup.DataFormat]), disk+"_") + for _, part := range capturedParts { + if part.Name == partName && b.skipMissingPart(err, missing, table, capturedDisk, partName, tableRemoteFile) { + missing.addFile(disk, archiveFile) + return nil + } + } + return errors.Wrapf(err, "%s is missing on remote storage and contains several parts (upload_by_part=false), can't skip it even with allow_missing_files_on_download=true", tableRemoteFile) + } return errors.Wrap(err, "DownloadCompressedStream") } atomic.AddUint64(&downloadedSize, uint64(downloadedBytes)) @@ -1063,6 +1132,9 @@ func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata. if len(manifestFiles) > 0 { pathSize, downloadErr := b.dst.DownloadPathWithManifest(dataCtx, partRemotePath, partLocalPath, manifestFiles, b.cfg.General.RetriesOnFailure, b.cfg.General.RetriesDuration, b.cfg.General.RetriesJitter, b, b.cfg.General.DownloadMaxBytesPerSecond) if downloadErr != nil { + if b.skipMissingPart(downloadErr, missing, table, capturedDisk, capturedPart.Name, partRemotePath) { + return os.RemoveAll(partLocalPath) + } return errors.WithMessage(downloadErr, "DownloadPathWithManifest") } atomic.AddUint64(&downloadedSize, uint64(pathSize)) @@ -1078,6 +1150,9 @@ func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata. // Fall back to Walk (ListObjectsV2) when no manifest is available pathSize, downloadErr := b.dst.DownloadPath(dataCtx, partRemotePath, partLocalPath, b.cfg.General.RetriesOnFailure, b.cfg.General.RetriesDuration, b.cfg.General.RetriesJitter, b, b.cfg.General.DownloadMaxBytesPerSecond) if downloadErr != nil { + if b.skipMissingPart(downloadErr, missing, table, capturedDisk, capturedPart.Name, partRemotePath) { + return os.RemoveAll(partLocalPath) + } return errors.Wrap(downloadErr, "DownloadPath") } atomic.AddUint64(&downloadedSize, uint64(pathSize)) @@ -1095,7 +1170,7 @@ func (b *Backuper) downloadTableData(ctx context.Context, remoteBackup metadata. if err := dataGroup.Wait(); err != nil { return 0, errors.Wrap(err, "one of downloadTableData go-routine return error") } - if isRebalancedAfterHardLinks.Load() { + if missing.apply(&table) || isRebalancedAfterHardLinks.Load() { if _, saveErr := table.Save(table.LocalFile, false); saveErr != nil { return 0, errors.Wrap(saveErr, "save rebalanced table after hardlinks") } @@ -1697,6 +1772,7 @@ func (b *Backuper) downloadDiffParts(ctx context.Context, remoteBackup metadata. downloadedDiffParts := uint32(0) downloadDiffGroup, downloadDiffCtx := errgroup.WithContext(ctx) downloadDiffGroup.SetLimit(int(b.cfg.General.DownloadConcurrency)) + missing := &missingParts{} diffRemoteFilesCache := map[string]*sync.Mutex{} diffRemoteFilesLock := &sync.Mutex{} isRebalancedAfterHardLinks := false @@ -1823,6 +1899,9 @@ func (b *Backuper) downloadDiffParts(ctx context.Context, remoteBackup metadata. for tableRemoteFile, tableLocalDir := range tableRemoteFiles { fileDiffBytes, downloadErr := b.downloadDiffRemoteFile(downloadDiffCtx, diffRemoteFilesLock, diffRemoteFilesCache, tableRemoteFile, tableLocalDir) if downloadErr != nil { + if b.skipMissingPart(downloadErr, missing, table, capturedDisk, partForDownload.Name, tableRemoteFile) { + return nil + } return errors.Wrap(downloadErr, "downloadDiffRemoteFile") } downloadedPartPath := path.Join(tableLocalDir, partForDownload.Name) @@ -1872,7 +1951,7 @@ func (b *Backuper) downloadDiffParts(ctx context.Context, remoteBackup metadata. if err := downloadDiffGroup.Wait(); err != nil { return 0, errors.Wrap(err, "one of downloadDiffParts go-routine return error") } - if isRebalancedAfterHardLinks { + if missing.apply(&table) || isRebalancedAfterHardLinks { if _, saveErr := table.Save(table.LocalFile, false); saveErr != nil { return 0, errors.Wrap(saveErr, "save rebalanced table after hardlinks in downloadDiffParts") } diff --git a/pkg/backup/download_test.go b/pkg/backup/download_test.go index 9ebfe8c27..8ba6ef6d1 100644 --- a/pkg/backup/download_test.go +++ b/pkg/backup/download_test.go @@ -2,7 +2,6 @@ package backup import ( "context" - "errors" "os" "path" "regexp" @@ -97,26 +96,6 @@ var remoteBackup = storage.Backup{ UploadDate: time.Now(), } -func TestIsRemoteMetadataNotFound(t *testing.T) { - notFoundMessages := []string{ - "object doesn't exist", - "key not found: metadata/default/test.json", - "NoSuchKey: The specified key does not exist", - "operation error S3: GetObject, https response error StatusCode: 404", - "StatusCode 404", - // real backend phrasings observed in test/integration TestMetadataNotFound* - "550 /backup/metadata/default/test.json: No such file or directory", // FTP - "file does not exist", // SFTP - "AzureBlob GetFileReaderAbsolute Download: RESPONSE ERROR (ServiceCode=BlobNotFound) RESPONSE Status: 404 The specified blob does not exist.", // Azure - } - for _, msg := range notFoundMessages { - assert.True(t, isRemoteMetadataNotFound(errors.New(msg)), msg) - } - - assert.False(t, isRemoteMetadataNotFound(nil)) - assert.False(t, isRemoteMetadataNotFound(errors.New("temporary network timeout"))) -} - func TestResumeExistingBackupMissingStateFileReturnsError(t *testing.T) { backupName := "test_resume_crash" defaultDataPath := t.TempDir() diff --git a/pkg/config/config.go b/pkg/config/config.go index 522322772..d57352112 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -100,6 +100,11 @@ type GeneralConfig struct { // DownloadDiskLimit - refuse `download` and `restore_remote` when usage of any local disk would exceed this percent (1..100) after download, // 0 (default) disables the check, `--disk-limit` CLI argument overrides it per command, see https://github.com/Altinity/clickhouse-backup/issues/1458 DownloadDiskLimit int `yaml:"download_disk_limit" envconfig:"DOWNLOAD_DISK_LIMIT"` + // AllowMissingFilesOnDownload - salvage mode for partially corrupted remote backups: when a data part file + // is missing on remote storage (404/NoSuchKey/BlobNotFound) `download` and `restore_remote` skip the part with + // an error-level log and drop it from the local table metadata instead of failing, metadata files are never skipped, + // `--allow-missing-files` CLI argument overrides it per command, see https://github.com/Altinity/clickhouse-backup/issues/1456 + AllowMissingFilesOnDownload bool `yaml:"allow_missing_files_on_download" envconfig:"ALLOW_MISSING_FILES_ON_DOWNLOAD"` // MaxBrokenPartRatio - maximum allowed fraction (0..1) of broken data parts that still produces a // successful but partial backup during backup creation (`create`, and the create stage of // `create_remote`). 0 (default) preserves legacy behavior where any broken part aborts the whole @@ -918,6 +923,7 @@ func DefaultConfig() *Config { CompressionUseMultiThread: true, MaxBrokenPartRatio: 0, DownloadDiskLimit: 0, + AllowMissingFilesOnDownload: false, }, ClickHouse: ClickHouseConfig{ Username: "default", @@ -1057,6 +1063,10 @@ func GetConfigFromCli(ctx *cli.Command) *Config { if ctx.Bool("rebind-replica-path-if-exists") { cfg.ClickHouse.RebindReplicaPathIfExists = true } + // `download`/`restore_remote` expose --allow-missing-files, same only-override-when-true semantics, see issues/1456 + if ctx.Bool("allow-missing-files") { + cfg.General.AllowMissingFilesOnDownload = true + } return cfg } diff --git a/pkg/server/server.go b/pkg/server/server.go index 04ea1d41c..3db6992f1 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -2347,6 +2347,11 @@ func (api *APIServer) httpRestoreRemoteHandler(w http.ResponseWriter, r *http.Re hardlinkExistsFiles = true fullCommand += " --hardlink-exists-files" } + // https://github.com/Altinity/clickhouse-backup/issues/1456 + if _, exist := api.getQueryParameter(query, "allow_missing_files"); exist { + cfg.General.AllowMissingFilesOnDownload = true + fullCommand += " --allow-missing-files" + } // https://github.com/Altinity/clickhouse-backup/issues/1458 diskLimit := 0 if v, exist := api.getQueryParameter(query, "disk_limit"); exist { @@ -2518,6 +2523,11 @@ func (api *APIServer) httpDownloadHandler(w http.ResponseWriter, r *http.Request hardlinkExistsFiles = true fullCommand += " --hardlink-exists-files" } + // https://github.com/Altinity/clickhouse-backup/issues/1456 + if _, exist := api.getQueryParameter(query, "allow_missing_files"); exist { + cfg.General.AllowMissingFilesOnDownload = true + fullCommand += " --allow-missing-files" + } // https://github.com/Altinity/clickhouse-backup/issues/1458 diskLimit := 0 if v, exist := api.getQueryParameter(query, "disk_limit"); exist { diff --git a/pkg/storage/not_found_error.go b/pkg/storage/not_found_error.go new file mode 100644 index 000000000..5cf9caacd --- /dev/null +++ b/pkg/storage/not_found_error.go @@ -0,0 +1,65 @@ +package storage + +import ( + "errors" + "io/fs" + "net/http" + "strings" + + "cloud.google.com/go/storage" + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror" + "github.com/aws/smithy-go" + smithyhttp "github.com/aws/smithy-go/transport/http" + "google.golang.org/api/googleapi" +) + +// IsNotFoundErr reports whether err means the remote object is permanently missing +// (S3 NoSuchKey/404, GCS ErrObjectNotExist/404, Azure BlobNotFound/404, FTP 550, SFTP/local fs.ErrNotExist), +// so retrying can never succeed, see https://github.com/Altinity/clickhouse-backup/issues/1456 +func IsNotFoundErr(err error) bool { + if err == nil { + return false + } + if errors.Is(err, ErrNotFound) || errors.Is(err, fs.ErrNotExist) || errors.Is(err, storage.ErrObjectNotExist) { + return true + } + var apiErr smithy.APIError + if errors.As(err, &apiErr) { + switch apiErr.ErrorCode() { + case "NoSuchKey", "NotFound": + return true + } + } + var httpErr *smithyhttp.ResponseError + if errors.As(err, &httpErr) && httpErr.HTTPStatusCode() == http.StatusNotFound { + return true + } + var gcpErr *googleapi.Error + if errors.As(err, &gcpErr) && gcpErr.Code == http.StatusNotFound { + return true + } + var azErr *azcore.ResponseError + if errors.As(err, &azErr) && (azErr.StatusCode == http.StatusNotFound || azErr.ErrorCode == string(bloberror.BlobNotFound)) { + return true + } + // every backend phrases "object is missing" differently and some wrap it as a plain string, + // so fall back to the known permanent-not-found markers across S3/GCS/Azure/FTP/SFTP/FS + message := strings.ToLower(err.Error()) + for _, marker := range []string{ + "doesn't exist", // GCS + "does not exist", // SFTP ("file does not exist"), Azure ("the specified blob does not exist") + "no such file or directory", // FTP (550), local filesystem + "key not found", + "nosuchkey", // S3 + "blobnotfound", // Azure Blob (x-ms-error-code) + "statuscode 404", // S3 SDK v2 + "statuscode: 404", + "status: 404", // Azure ("RESPONSE Status: 404") + } { + if strings.Contains(message, marker) { + return true + } + } + return false +} diff --git a/pkg/storage/not_found_error_test.go b/pkg/storage/not_found_error_test.go new file mode 100644 index 000000000..967937d54 --- /dev/null +++ b/pkg/storage/not_found_error_test.go @@ -0,0 +1,52 @@ +package storage + +import ( + "errors" + "io/fs" + "net/http" + "testing" + + gcs "cloud.google.com/go/storage" + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror" + "github.com/aws/smithy-go" + smithyhttp "github.com/aws/smithy-go/transport/http" + pkgerrors "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "google.golang.org/api/googleapi" +) + +func TestIsNotFoundErr(t *testing.T) { + notFoundMessages := []string{ + "object doesn't exist", + "key not found: metadata/default/test.json", + "NoSuchKey: The specified key does not exist", + "operation error S3: GetObject, https response error StatusCode: 404", + "StatusCode 404", + // real backend phrasings observed in test/integration TestMetadataNotFound* + "550 /backup/metadata/default/test.json: No such file or directory", // FTP + "file does not exist", // SFTP + "AzureBlob GetFileReaderAbsolute Download: RESPONSE ERROR (ServiceCode=BlobNotFound) RESPONSE Status: 404 The specified blob does not exist.", // Azure + } + for _, msg := range notFoundMessages { + assert.True(t, IsNotFoundErr(errors.New(msg)), msg) + } + + // typed errors from SDKs and wrapped sentinels + assert.True(t, IsNotFoundErr(NewErrNotFound("metadata/default/test.json"))) + assert.True(t, IsNotFoundErr(pkgerrors.Wrap(NewErrNotFound("k"), "DownloadCompressedStream StatFile"))) + assert.True(t, IsNotFoundErr(pkgerrors.Wrap(fs.ErrNotExist, "sftp"))) + assert.True(t, IsNotFoundErr(gcs.ErrObjectNotExist)) + assert.True(t, IsNotFoundErr(&smithy.GenericAPIError{Code: "NoSuchKey", Message: "x"})) + assert.True(t, IsNotFoundErr(&smithyhttp.ResponseError{Response: &smithyhttp.Response{Response: &http.Response{StatusCode: 404}}, Err: errors.New("x")})) + assert.True(t, IsNotFoundErr(&googleapi.Error{Code: 404})) + assert.True(t, IsNotFoundErr(&azcore.ResponseError{StatusCode: 404})) + assert.True(t, IsNotFoundErr(&azcore.ResponseError{ErrorCode: string(bloberror.BlobNotFound)})) + + assert.False(t, IsNotFoundErr(nil)) + assert.False(t, IsNotFoundErr(errors.New("temporary network timeout"))) + assert.False(t, IsNotFoundErr(&smithy.GenericAPIError{Code: "SlowDown"})) + assert.False(t, IsNotFoundErr(&smithyhttp.ResponseError{Response: &smithyhttp.Response{Response: &http.Response{StatusCode: 503}}, Err: errors.New("x")})) + assert.False(t, IsNotFoundErr(&googleapi.Error{Code: 503})) + assert.False(t, IsNotFoundErr(&azcore.ResponseError{StatusCode: 500})) +} diff --git a/test/integration/testAllowMissingFiles_test.go b/test/integration/testAllowMissingFiles_test.go new file mode 100644 index 000000000..256319479 --- /dev/null +++ b/test/integration/testAllowMissingFiles_test.go @@ -0,0 +1,114 @@ +//go:build integration + +package main + +import ( + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/rs/zerolog/log" +) + +// TestAllowMissingFilesOnDownload reproduces https://github.com/Altinity/clickhouse-backup/issues/1456: +// - upload a backup with two parts (upload_by_part=true, one archive per part), +// - delete one part archive on remote storage (MinIO via `mc`), +// - `download` without the flag must fail fast with a not-found error instead of burning the retry backoff, +// - `download --allow-missing-files` must succeed, skip the missing part with an error log, +// drop it from the local table metadata, and `restore` must bring back the surviving part only. +func TestAllowMissingFilesOnDownload(t *testing.T) { + chVer := strings.ReplaceAll(os.Getenv("CLICKHOUSE_VERSION"), ".", "_") + env, r := NewTestEnvironment(t) + env.connectWithWait(t, r, 0*time.Second, 1*time.Second, 1*time.Minute) + defer env.Cleanup(t, r) + + r.NoError(env.DockerCP("configs/config-s3.yml", "clickhouse-backup:/etc/clickhouse-backup/config.yml")) + cfgPath, _ := env.resolveConfigPaths(r, "config-s3.yml") + // MinIO only sees objects that went through its S3 API, so use `mc`; config path is relative to bucket + root := "local/clickhouse/" + cfgPath + const mcAliasCmd = "mc alias set local https://localhost:9000 access_key it_is_my_super_secret_key >/dev/null 2>&1" + + tableShort := "test_allow_missing_files" + tableName := "default." + tableShort + backupName := fmt.Sprintf("allow_missing_files_%s_%d", chVer, time.Now().UnixNano()) + // two partitions -> two parts -> two archives, background merges never cross partitions + env.queryWithNoError(t, r, fmt.Sprintf("CREATE TABLE IF NOT EXISTS %s(id UInt64, p UInt8) ENGINE=MergeTree() PARTITION BY p ORDER BY id", tableName)) + env.queryWithNoError(t, r, fmt.Sprintf("INSERT INTO %s SELECT number, 1 FROM numbers(100)", tableName)) + env.queryWithNoError(t, r, fmt.Sprintf("INSERT INTO %s SELECT number, 2 FROM numbers(50)", tableName)) + t.Cleanup(func() { + dropQ := "DROP TABLE IF EXISTS " + tableName + if compareVersion(os.Getenv("CLICKHOUSE_VERSION"), "20.3") > 0 { + dropQ += " NO DELAY" + } + if _, err := env.DockerExecOut("clickhouse", "clickhouse", "client", "-q", dropQ); err != nil { + log.Warn().Err(err).Str("table", tableName).Msg("t.Cleanup: failed to drop table") + } + }) + defer func() { + for _, cmd := range [][]string{ + {"clickhouse-backup", "delete", "local", backupName}, + {"clickhouse-backup", "delete", "remote", backupName}, + } { + if out, err := env.DockerExecOut("clickhouse-backup", cmd...); err != nil { + log.Warn().Err(err).Str("cmd", strings.Join(cmd, " ")).Msgf("allowMissingFiles teardown: %s", out) + } + } + }() + + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "create", "--tables", tableName, backupName) + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "upload", backupName) + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "delete", "local", backupName) + + // partition 2 lives in part 2_*; delete exactly that archive on remote storage + shadowDir := fmt.Sprintf("%s/%s/shadow/default/%s/", root, backupName, tableShort) + lsOut, err := env.DockerExecOut("minio", "bash", "-c", fmt.Sprintf("%s && mc ls %s", mcAliasCmd, shadowDir)) + r.NoError(err, "mc ls %s: %s", shadowDir, lsOut) + // `mc ls` prints "[date] [time] [size] [name]" per object, the archive name is the last field + archives := make([]string, 0, 2) + missingArchive := "" + for _, line := range strings.Split(strings.TrimSpace(lsOut), "\n") { + fields := strings.Fields(line) + if len(fields) == 0 { + continue + } + f := fields[len(fields)-1] + archives = append(archives, f) + if strings.HasPrefix(f, "default_2_") { + missingArchive = f + } + } + r.NotEmpty(missingArchive, "expected an archive for partition 2 in %s, got: %s", shadowDir, lsOut) + r.Len(archives, 2, "expected exactly two part archives, got: %s", lsOut) + env.DockerExecNoError(r, "minio", "bash", "-c", fmt.Sprintf("%s && mc rm %s%s", mcAliasCmd, shadowDir, missingArchive)) + + log.Debug().Msg("download without --allow-missing-files must fail fast (no retry backoff)") + start := time.Now() + out, err := env.DockerExecOut("clickhouse-backup", "clickhouse-backup", "download", backupName) + elapsed := time.Since(start) + log.Debug().Msg(out) + r.Error(err, "download must fail when a part archive is missing on remote storage, output: %s", out) + r.Contains(strings.ToLower(out), "not found", "download must report the missing archive as not-found, output: %s", out) + // "Will wait near Ns and retry" (pkg/backup/backuper.go) is logged on every retry attempt + r.NotContains(out, "and retry", "download must not retry on a permanent 404, output: %s", out) + r.Less(elapsed, 20*time.Second, "download must not burn the retry backoff on a permanent 404, took %s, output: %s", elapsed, out) + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "delete", "local", backupName) + + log.Debug().Msg("download --allow-missing-files must skip the missing part and succeed") + out, err = env.DockerExecOut("clickhouse-backup", "clickhouse-backup", "download", "--allow-missing-files", backupName) + log.Debug().Msg(out) + r.NoError(err, "download --allow-missing-files must succeed, output: %s", out) + r.Contains(out, "skip it because allow_missing_files_on_download=true", "missing part must be reported with an error log, output: %s", out) + r.Contains(out, "1 data parts were missing on remote storage and skipped", "summary must count the skipped part, output: %s", out) + r.NotContains(out, "and retry", "download must not retry on a permanent 404, output: %s", out) + + localMeta, err := env.DockerExecOut("clickhouse-backup", "cat", fmt.Sprintf("/var/lib/clickhouse/backup/%s/metadata/default/%s.json", backupName, tableShort)) + r.NoError(err, "read local table metadata: %s", localMeta) + r.NotContains(localMeta, missingArchive, "missing archive must be dropped from table metadata files: %s", localMeta) + r.NotContains(localMeta, `"name": "2_`, "missing part must be dropped from table metadata parts: %s", localMeta) + r.Contains(localMeta, `"name": "1_`, "surviving part must stay in table metadata: %s", localMeta) + + env.DockerExecNoError(r, "clickhouse-backup", "clickhouse-backup", "restore", "--rm", backupName) + env.checkCount(r, 1, 100, fmt.Sprintf("SELECT count() FROM %s SETTINGS empty_result_for_aggregation_by_empty_set=0", tableName)) +} diff --git a/test/integration/testMetadataNotFound_test.go b/test/integration/testMetadataNotFound_test.go index 748acacd7..3c0ca9d79 100644 --- a/test/integration/testMetadataNotFound_test.go +++ b/test/integration/testMetadataNotFound_test.go @@ -159,7 +159,7 @@ func runMetadataNotFoundScenario(t *testing.T, tc metadataNotFoundCase) { r.Error(err, "download must fail when remote table metadata is missing, output: %s", out) // The fail-fast path emits this distinctive message; the raw backend errors // (e.g. "no such file or directory", "BlobNotFound") do not contain it, so a - // match proves isRemoteMetadataNotFound classified the 404 correctly. + // match proves storage.IsNotFoundErr classified the 404 correctly. r.Contains(strings.ToLower(out), "not found on remote storage", "download must report the missing metadata as not-found, output: %s", out) // "Will wait near Ns and retry" (pkg/backup/backuper.go) is logged on every // retry attempt; its absence proves the permanent 404 broke out immediately. diff --git a/test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot b/test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot index d732345dc..3ca593493 100644 --- a/test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot +++ b/test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot @@ -1,4 +1,4 @@ -default_config = r"""'[\'general:\', \' remote_storage: none\', \' backups_to_keep_local: 0\', \' backups_to_keep_remote: 0\', \' log_level: info\', \' disable_environment_override: false\', \' allow_empty_backups: false\', \' rebase_before_remove_old_remote: false\', \' rebase_during_delete: false\', \' download_disk_limit: 0\', \' pipe_buffer_size: 131072\', \' download_copy_buffer_size: 0\', \' compression_use_multi_thread: true\', \' compression_threads: 0\', \' compression_buffer_size: 0\', \' allow_object_disk_streaming: false\', \' use_resumable_state: true\', \' restore_schema_on_cluster: ""\', \' upload_by_part: true\', \' download_by_part: true\', \' restore_database_mapping: {}\', \' restore_table_mapping: {}\', \' retries_on_failure: 3\', \' retries_pause: 5s\', \' retries_jitter: 0\', \' watch_interval: 1h\', \' full_interval: 24h\', \' watch_backup_name_template: shard{shard}-{type}-{time:20060102150405}\', \' callback_url: ""\', \' callback_timeout: 5s\', \' watch_schedules: []\', \' sharded_operation_mode: ""\', \' cpu_nice_priority: 15\', \' io_nice_priority: idle\', \' rbac_backup_always: true\', \' rbac_conflict_resolution: recreate\', \' config_backup_always: false\', \' named_collections_backup_always: false\', \' delete_batch_size: 1000\', \' status_history_size: 1000\', \' retriesduration: 5s\', \' watchduration: 1h0m0s\', \' fullduration: 24h0m0s\', \' callbacktimeoutduration: 5s\', \'clickhouse:\', \' username: default\', \' password: ""\', \' host: localhost\', \' port: 9000\', \' disk_mapping: {}\', \' skip_tables:\', \' - system.*\', \' - INFORMATION_SCHEMA.*\', \' - information_schema.*\', \' - _temporary_and_external_tables.*\', \' skip_table_engines: []\', \' skip_disks: []\', \' skip_disk_types: []\', \' timeout: 30m\', \' freeze_by_part: false\', \' freeze_by_part_where: ""\', \' use_embedded_backup_restore: false\', \' use_embedded_backup_restore_cluster: ""\', \' embedded_backup_disk: ""\', \' backup_mutations: true\', \' restore_as_attach: false\', \' restore_distributed_cluster: ""\', \' check_parts_columns: true\', \' parts_columns_batch_size: 25\', \' secure: false\', \' skip_verify: false\', \' sync_replicated_tables: false\', \' log_sql_queries: true\', \' config_dir: /etc/clickhouse-server/\', \' restart_command: exec:systemctl restart clickhouse-server\', \' ignore_not_exists_error_during_freeze: true\', \' check_replicas_before_attach: true\', \' default_replica_path: /clickhouse/tables/{cluster}/{shard}/{database}/{table}\', " default_replica_name: \'{replica}\'", \' rebind_replica_path_if_exists: false\', \' tls_key: ""\', \' tls_cert: ""\', \' tls_ca: ""\', \' debug: false\', \' force_rebalance: false\', \'s3:\', \' access_key: ""\', \' secret_key: ""\', \' bucket: ""\', \' endpoint: ""\', \' region: us-east-1\', \' acl: private\', \' assume_role_arn: ""\', \' force_path_style: false\', \' path: ""\', \' object_disk_path: ""\', \' disable_ssl: false\', \' compression_level: 1\', \' compression_format: tar\', \' sse: ""\', \' sse_kms_key_id: ""\', \' sse_customer_algorithm: ""\', \' sse_customer_key: ""\', \' sse_customer_key_md5: ""\', \' sse_kms_encryption_context: ""\', \' disable_cert_verification: false\', \' use_custom_storage_class: false\', \' storage_class: STANDARD\', \' custom_storage_class_map: {}\', \' allow_multipart_download: false\', \' object_labels: {}\', \' request_payer: ""\', \' check_sum_algorithm: ""\', \' request_content_md5: false\', \' retry_mode: standard\', \' chunk_size: 5242880\', \' delete_batch_min_size: 0\', \' delete_batch_fallback_to_single: true\', \' debug: false\', \' http_write_buffer_size: 0\', \' http_read_buffer_size: 0\', \' http_idle_conn_timeout: ""\', \' http2_send_ping_timeout: 30s\', \' http2_ping_timeout: 15s\', \' http2_write_byte_timeout: 60s\', \'gcs:\', \' credentials_file: ""\', \' credentials_json: ""\', \' credentials_json_encoded: ""\', \' sa_email: ""\', \' embedded_access_key: ""\', \' embedded_secret_key: ""\', \' skip_credentials: false\', \' bucket: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_level: 1\', \' compression_format: tar\', \' debug: false\', \' force_http: false\', \' disable_http2: false\', \' endpoint: ""\', \' storage_class: STANDARD\', \' object_labels: {}\', \' custom_storage_class_map: {}\', \' chunk_size: 16777216\', \' encryption_key: ""\', \' upload_buffer_size: 131072\', \' allow_multipart_upload: false\', \' multipart_upload_min_size: 1073741824\', \' allow_multipart_download: false\', \'cos:\', \' url: ""\', \' timeout: 2m\', \' secret_id: ""\', \' secret_key: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' allow_multipart_download: false\', \' debug: false\', \'api:\', \' listen: localhost:7171\', \' enable_metrics: true\', \' enable_pprof: false\', \' username: ""\', \' password: ""\', \' secure: false\', \' certificate_file: ""\', \' private_key_file: ""\', \' ca_cert_file: ""\', \' ca_key_file: ""\', \' create_integration_tables: false\', \' integration_tables_host: ""\', \' allow_parallel: false\', \' complete_resumable_after_restart: true\', \' complete_resumable_after_restart_commands:\', \' - upload\', \' - download\', \' watch_is_main_process: false\', \' backup_actions_skip_commands: []\', \' cancel_operation_timeout: 1800s\', \'ftp:\', \' address: ""\', \' timeout: 2m\', \' username: ""\', \' password: ""\', \' tls: false\', \' skip_tls_verify: false\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' debug: false\', \'sftp:\', \' address: ""\', \' port: 22\', \' username: ""\', \' password: ""\', \' key: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' debug: false\', \'azblob:\', \' endpoint_schema: https\', \' endpoint_suffix: core.windows.net\', \' account_name: ""\', \' account_key: ""\', \' sas: ""\', \' use_managed_identity: false\', \' container: ""\', \' assume_container_exists: false\', \' path: ""\', \' object_disk_path: ""\', \' compression_level: 1\', \' compression_format: tar\', \' sse_key: ""\', \' buffer_count: 3\', \' timeout: 4h\', \' debug: false\', \'custom:\', \' upload_command: ""\', \' download_command: ""\', \' list_command: ""\', \' delete_command: ""\', \' command_timeout: 4h\', \' commandtimeoutduration: 4h0m0s\']'""" +default_config = r"""'[\'general:\', \' remote_storage: none\', \' backups_to_keep_local: 0\', \' backups_to_keep_remote: 0\', \' log_level: info\', \' disable_environment_override: false\', \' allow_empty_backups: false\', \' rebase_before_remove_old_remote: false\', \' rebase_during_delete: false\', \' download_disk_limit: 0\', \' allow_missing_files_on_download: false\', \' pipe_buffer_size: 131072\', \' download_copy_buffer_size: 0\', \' compression_use_multi_thread: true\', \' compression_threads: 0\', \' compression_buffer_size: 0\', \' allow_object_disk_streaming: false\', \' use_resumable_state: true\', \' restore_schema_on_cluster: ""\', \' upload_by_part: true\', \' download_by_part: true\', \' restore_database_mapping: {}\', \' restore_table_mapping: {}\', \' retries_on_failure: 3\', \' retries_pause: 5s\', \' retries_jitter: 0\', \' watch_interval: 1h\', \' full_interval: 24h\', \' watch_backup_name_template: shard{shard}-{type}-{time:20060102150405}\', \' callback_url: ""\', \' callback_timeout: 5s\', \' watch_schedules: []\', \' sharded_operation_mode: ""\', \' cpu_nice_priority: 15\', \' io_nice_priority: idle\', \' rbac_backup_always: true\', \' rbac_conflict_resolution: recreate\', \' config_backup_always: false\', \' named_collections_backup_always: false\', \' delete_batch_size: 1000\', \' status_history_size: 1000\', \' retriesduration: 5s\', \' watchduration: 1h0m0s\', \' fullduration: 24h0m0s\', \' callbacktimeoutduration: 5s\', \'clickhouse:\', \' username: default\', \' password: ""\', \' host: localhost\', \' port: 9000\', \' disk_mapping: {}\', \' skip_tables:\', \' - system.*\', \' - INFORMATION_SCHEMA.*\', \' - information_schema.*\', \' - _temporary_and_external_tables.*\', \' skip_table_engines: []\', \' skip_disks: []\', \' skip_disk_types: []\', \' timeout: 30m\', \' freeze_by_part: false\', \' freeze_by_part_where: ""\', \' use_embedded_backup_restore: false\', \' use_embedded_backup_restore_cluster: ""\', \' embedded_backup_disk: ""\', \' backup_mutations: true\', \' restore_as_attach: false\', \' restore_distributed_cluster: ""\', \' check_parts_columns: true\', \' parts_columns_batch_size: 25\', \' secure: false\', \' skip_verify: false\', \' sync_replicated_tables: false\', \' log_sql_queries: true\', \' config_dir: /etc/clickhouse-server/\', \' restart_command: exec:systemctl restart clickhouse-server\', \' ignore_not_exists_error_during_freeze: true\', \' check_replicas_before_attach: true\', \' default_replica_path: /clickhouse/tables/{cluster}/{shard}/{database}/{table}\', " default_replica_name: \'{replica}\'", \' rebind_replica_path_if_exists: false\', \' tls_key: ""\', \' tls_cert: ""\', \' tls_ca: ""\', \' debug: false\', \' force_rebalance: false\', \'s3:\', \' access_key: ""\', \' secret_key: ""\', \' bucket: ""\', \' endpoint: ""\', \' region: us-east-1\', \' acl: private\', \' assume_role_arn: ""\', \' force_path_style: false\', \' path: ""\', \' object_disk_path: ""\', \' disable_ssl: false\', \' compression_level: 1\', \' compression_format: tar\', \' sse: ""\', \' sse_kms_key_id: ""\', \' sse_customer_algorithm: ""\', \' sse_customer_key: ""\', \' sse_customer_key_md5: ""\', \' sse_kms_encryption_context: ""\', \' disable_cert_verification: false\', \' use_custom_storage_class: false\', \' storage_class: STANDARD\', \' custom_storage_class_map: {}\', \' allow_multipart_download: false\', \' object_labels: {}\', \' request_payer: ""\', \' check_sum_algorithm: ""\', \' request_content_md5: false\', \' retry_mode: standard\', \' chunk_size: 5242880\', \' delete_batch_min_size: 0\', \' delete_batch_fallback_to_single: true\', \' debug: false\', \' http_write_buffer_size: 0\', \' http_read_buffer_size: 0\', \' http_idle_conn_timeout: ""\', \' http2_send_ping_timeout: 30s\', \' http2_ping_timeout: 15s\', \' http2_write_byte_timeout: 60s\', \'gcs:\', \' credentials_file: ""\', \' credentials_json: ""\', \' credentials_json_encoded: ""\', \' sa_email: ""\', \' embedded_access_key: ""\', \' embedded_secret_key: ""\', \' skip_credentials: false\', \' bucket: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_level: 1\', \' compression_format: tar\', \' debug: false\', \' force_http: false\', \' disable_http2: false\', \' endpoint: ""\', \' storage_class: STANDARD\', \' object_labels: {}\', \' custom_storage_class_map: {}\', \' chunk_size: 16777216\', \' encryption_key: ""\', \' upload_buffer_size: 131072\', \' allow_multipart_upload: false\', \' multipart_upload_min_size: 1073741824\', \' allow_multipart_download: false\', \'cos:\', \' url: ""\', \' timeout: 2m\', \' secret_id: ""\', \' secret_key: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' allow_multipart_download: false\', \' debug: false\', \'api:\', \' listen: localhost:7171\', \' enable_metrics: true\', \' enable_pprof: false\', \' username: ""\', \' password: ""\', \' secure: false\', \' certificate_file: ""\', \' private_key_file: ""\', \' ca_cert_file: ""\', \' ca_key_file: ""\', \' create_integration_tables: false\', \' integration_tables_host: ""\', \' allow_parallel: false\', \' complete_resumable_after_restart: true\', \' complete_resumable_after_restart_commands:\', \' - upload\', \' - download\', \' watch_is_main_process: false\', \' backup_actions_skip_commands: []\', \' cancel_operation_timeout: 1800s\', \'ftp:\', \' address: ""\', \' timeout: 2m\', \' username: ""\', \' password: ""\', \' tls: false\', \' skip_tls_verify: false\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' debug: false\', \'sftp:\', \' address: ""\', \' port: 22\', \' username: ""\', \' password: ""\', \' key: ""\', \' path: ""\', \' object_disk_path: ""\', \' compression_format: tar\', \' compression_level: 1\', \' debug: false\', \'azblob:\', \' endpoint_schema: https\', \' endpoint_suffix: core.windows.net\', \' account_name: ""\', \' account_key: ""\', \' sas: ""\', \' use_managed_identity: false\', \' container: ""\', \' assume_container_exists: false\', \' path: ""\', \' object_disk_path: ""\', \' compression_level: 1\', \' compression_format: tar\', \' sse_key: ""\', \' buffer_count: 3\', \' timeout: 4h\', \' debug: false\', \'custom:\', \' upload_command: ""\', \' download_command: ""\', \' list_command: ""\', \' delete_command: ""\', \' command_timeout: 4h\', \' commandtimeoutduration: 4h0m0s\']'""" help_flag = r"""'NAME:\n clickhouse-backup - Tool for easy backup of ClickHouse with cloud supportUSAGE:\n clickhouse-backup [-t, --tables=.
] DESCRIPTION:\n Run as \'root\' or \'clickhouse\' userCOMMANDS:\n tables List of tables, exclude skip_tables\n create Create new backup\n create_remote Create and upload new backup\n upload Upload backup to remote storage\n list List of backups\n download Download backup from remote storage\n rebase Copy required parts from `required_backup` chain into remote backup and remove `required_backup` dependency, so backup becomes full\n rebalance Move data parts inside local backup between disks to match current system.parts layout and storage policy, skip parts on object disks\n restore Create schema and restore data from backup\n restore_remote Download and restore\n restore_cloud Restore ClickHouse Cloud native S3 backup (Shared engines) as Atomic databases and Replicated*MergeTree tables on the current server\n delete Delete specific backup\n default-config Print default config\n print-config Print current config merged with environment variables\n clean Remove data in \'shadow\' folder from all \'path\' folders available from \'system.disks\'\n clean_remote_broken Remove all broken remote backups\n clean_local_broken Remove all broken local backups\n clean_broken_retention Remove orphan entries under remote `path` and `object_disks_path` that are not in the live backup list\n watch Run infinite loop which create full + incremental backup sequence to allow efficient backup sequences\n acvp Run ACVP wrapper protocol over stdin/stdout\n server Run API server\n help, h Shows a list of commands or help for one commandGLOBAL OPTIONS:\n --config string, -c string Config \'FILE\' name. (default: "/etc/clickhouse-backup/config.yml") [$CLICKHOUSE_BACKUP_CONFIG]\n --environment-override string, --env string [ --environment-override string, --env string ] override any environment variable via CLI parameter\n --fips-info Display FIPS build/runtime info and exit (no Go toolchain required).\n --help, -h show help\n --version, -v print the version'""" From db6b3ea52d9d8a0e88de93323c8742326d3d34a3 Mon Sep 17 00:00:00 2001 From: slach Date: Thu, 10 Sep 2026 09:46:33 +0500 Subject: [PATCH 3/3] Add server log tail and readiness polling to TestKill* API calls TestKillRestore failed in CI with a bare `curl: (7) Connection refused` from postAction and nothing about why the server was not listening. Reuse waitForAPIServerReady instead of a fixed 3s sleep after `clickhouse-backup server` starts, and attach the server log tail to postAction failures so the next occurrence shows the actual cause. Co-Authored-By: Claude Fable 5.1 --- test/integration/kill_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/kill_test.go b/test/integration/kill_test.go index a8e916719..062f33d46 100644 --- a/test/integration/kill_test.go +++ b/test/integration/kill_test.go @@ -145,7 +145,7 @@ func TestKillUpload(t *testing.T) { t.Errorf("TestKillUpload teardown: drop database %s, error=%+v", dbName, err) } }() - time.Sleep(3 * time.Second) + waitForAPIServerReady(r, env, 30*time.Second) pidPath := fmt.Sprintf("/tmp/clickhouse-backup.%s.pid", backupName) @@ -266,7 +266,7 @@ func TestKillDownload(t *testing.T) { t.Errorf("TestKillDownload teardown: drop database %s, error=%+v", dbName, err) } }() - time.Sleep(3 * time.Second) + waitForAPIServerReady(r, env, 30*time.Second) // 1. create local backup, push it remote, then drop local so download works. runActionWait(r, env, fmt.Sprintf("create --tables=%s.* %s", dbName, backupName), "create", backupName, 60*time.Second) @@ -317,7 +317,7 @@ func TestKillCreate(t *testing.T) { t.Errorf("TestKillCreate teardown: drop database %s, error=%+v", dbName, err) } }() - time.Sleep(3 * time.Second) + waitForAPIServerReady(r, env, 30*time.Second) // start happens inside observeInProgressAndKill so a fast create cannot // finish before the kill is issued. @@ -354,7 +354,7 @@ func TestKillRestore(t *testing.T) { t.Errorf("TestKillRestore teardown: drop database %s, error=%+v", dbName, err) } }() - time.Sleep(3 * time.Second) + waitForAPIServerReady(r, env, 30*time.Second) // create a local backup, drop the table so restore has to recreate+attach. var fullRows uint64 @@ -437,7 +437,7 @@ func postAction(r *require.Assertions, env *TestEnvironment, command string) str body := fmt.Sprintf(`{"command":%q}`, command) out, err := env.DockerExecOut("clickhouse-backup", "bash", "-ce", execCurlWithFailBody("-XPOST 'http://127.0.0.1:7171/backup/actions' -d '"+body+"'")) - r.NoError(err, "%s\nPOST /backup/actions %q error: %v", out, command, err) + r.NoError(err, "%s\nPOST /backup/actions %q error: %v\n%s", out, command, err, apiServerLogTailOnError(env, err)) return out }