Repository navigation
Conversation
The keys of <command>.state2 are relative to the remote storage path and the params stored in the state held only CLI arguments. After an interrupted run, changing s3.path (or bucket, endpoint, remote_storage, object_disk_path, compression_format) and re-running with --resume skipped every file already recorded for the old location, leaving an incomplete backup on the new one. For restore, keys are named after the backup tables, so changing restore_database_mapping or restore_table_mapping skipped tables that were never restored into the new targets. Store a configHash param computed from only those fields, so the existing params-mismatch cleanup discards the state. Concurrency, retries, credentials, log level and other tuning still keep resume. The mapping fields are hashed only for restore and download_restore_streaming. ResumeOperationsAfterRestart is unchanged: it reads stored params by name and ignores configHash. The cleanup log is raised from Info to Warn so discarding state is visible. Fixes #1010 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-up for #1010. ResumeOperationsAfterRestart rebuilt restore / restore_remote --streaming argv without the mapping flags, so with the mappings in the config hash a resumed mapped restore wiped its state and restored into the original tables. The merged mappings are now stored as plain params and passed back as --restore-database-mapping / --restore-table-mapping. Customer-provided encryption keys (s3 sse_customer_key, gcs encryption_key, azblob sse_key) are hashed: resuming after a key rotation completed a backup no single key can read. sse and sse_kms_key_id stay out, server-side keys keep old objects readable. Hash inputs are now per command: compression_format only for upload and create_upload_streaming (download and restore use data_format of the backup), storage path not for create (object disk copy writes under object_disk_path). Mappings stay for non-streaming restore too: its state keys are source paths, but the object keys of a mapped table carry the destination table (#1568), so skipping on a changed mapping would attach objects shared with the previous target. Only a configHash change is logged as Warn, routine resets such as the needClean nonce stay Info, and only the differing params are logged. The unreachable custom storage branch is removed, max_file_size is dropped from the must-keep test set, ChangeLog gets an upgrade note about the one-time reset of states written by older versions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coverage Report for CI Build 39210945387Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage at 70.2% (no base build to compare)Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. 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
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.
Fixes #1010
Problem
Resumable state (
backup/<name>/<command>.state2) was reset only when CLI parameters changed (#840). If the config changed between an interrupted run and--resume— another bucket/path/remote_storage, a rotated customer encryption key, or different restore mappings — resume skipped work that was done for the old target, producing an incomplete or unreadable backup/restore.Change
resumableConfigHash(cfg, command): sha256 over only the config fields that define what state keys refer to, stored asparams["configHash"]at everyNewStatecall site; the existing params-mismatch cleanup wipes the state when it differs.remote_storage, active storage location (s3/gcs: endpoint, bucket, path, object_disk_path; azblob: endpoint_suffix, account_name, container, path, object_disk_path; cos: url, path, object_disk_path; ftp/sftp: address(, port), path, object_disk_path) and customer-provided keys (s3.sse_customer_key,gcs.encryption_key,azblob.sse_key; only the digest is stored).upload/create_upload_streaming; storagepathexcluded forcreate; restore mappings forrestoreanddownload_restore_streaming(mapped object keys depend on the destination table, see Dropping a mapped restore deletes objects needed by the original S3-backed table (reproduced on 2.8.1) #1568).sse/sse_kms_key_id.ResumeOperationsAfterRestartnow replays restore mappings (stored as plain params) so a resumed restore uses the same mapping.Notes
--resume(documented in ChangeLog).upload_by_part: falsethe size-based archive split can change between runs (max_file_size, orCalculateMaxSizeafter merges), so resume may skip data; fixing it requires persisting the split plan.Tests
TestResumableConfigHashS3/GCS/AzureBlob,TestChangedParams,TestNewStateCleansStateWhenConfigHashChanges,TestResumableRestoreMappingArgs.TestResumableStateConfigChange(new),TestUploadStaleResumeStateAfterRemoteDelete,TestResumeOperationsAfterRestart{S3,Azure,SFTP,FTP,COS}pass (GCS skipped locally).🤖 Generated with Claude Code