Fail fast on missing remote objects, add allow_missing_files_on_download salvage mode - #1544
Merged
Conversation
…wnload 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 #1456 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
…to download_allow_missing_files
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 <noreply@anthropic.com>
Coverage Report for CI Build 34438492208Coverage increased (+0.2%) to 67.055%Details
Uncovered Changes
Coverage Regressions7 previously-covered lines in 3 files lost coverage.
Coverage Stats
💛 - Coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #1456
Problem
Backuper.Classifyretried every non-context error, so a permanently missing remote object (S3NoSuchKey/404, GCS 404, AzureBlobNotFound, FTP/SFTP not-found) burned the wholeretries_on_failurexretries_durationbackoff per file. #1379 fixed this only for table metadata.json/.sql; part data archives, directory-format parts andrequired_backupdiff parts still retried. There was also no way to salvage the intact parts of a partially corrupted remote backup.Changes
storage.IsNotFoundErr(err)— recognises not-found across backends:storage.ErrNotFound,fs.ErrNotExist, GCSErrObjectNotExist/googleapi404, smithyNoSuchKey/NotFound/HTTP 404, AzureBlobNotFound/404, plus the string markers previously inisRemoteMetadataNotFound(which is removed in favour of the shared helper).Backuper.Classifyreturnsretrier.Failfor not-found errors, so every retried remote operation fails fast.general.allow_missing_files_on_download(envALLOW_MISSING_FILES_ON_DOWNLOAD, defaultfalse),--allow-missing-filesfordownload/restore_remote,allow_missing_filesquery argument forPOST /backup/downloadandPOST /backup/restore_remote(same only-override-when-true semantics as--rebind-replica-path-if-exists).downloadTableData/downloadDiffParts: a part whose data is missing on remote is skipped with anerror-level log, dropped from local table metadata (partsandfiles), counted and reported in a final summary; partially downloaded directory-format parts are removed. Metadata files are never skipped. Withupload_by_part: falsea missing archive stays fatal with an explicit message, since one part can't be carved out of a size-based bundle.default-configsnapshot.Tests
TestIsNotFoundErr(typed SDK errors + string markers, negative cases),TestClassifyextended.TestAllowMissingFilesOnDownload(MinIO) — two-part backup,mc rmone archive,downloadfails fast (<20s, no "and retry"),download --allow-missing-filessucceeds, metadata drops the part,restorebrings back the surviving 100 rows.TestMetadataNotFoundS3|SFTP|FTPstill pass.🤖 Generated with Claude Code