Altinity / Altinity/clickhouse-backup

upload --resume writes an incomplete manifest.bolt.gz: files skipped as already processed are never recorded

Open
#1,569 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
1.6k
Forks
276
Avg merge
18h 3m
Merged PRs (30d)
26

Description

Summary

Since v2.8.0 upload writes manifest.bolt.gz (#1375) so that download and restore can avoid per-part Walk calls. On a resumed upload (upload --resume) every object the resumable state marks as already processed is skipped, and none of the skip paths record that object in the manifest. NewManifestWriter always starts empty and nothing seeds it from the previous attempt or from remote storage, so the manifest uploaded at the end of a resumed upload lists only the files transferred by that attempt. It then overwrites the (more complete) manifest written by the first attempt.

Where (pkg/backup/upload.go on master, 2026-09-17)

Skip paths that return before recordUploadedFile / recordUploadedFiles is reached:

  • uploadTableData, directory format: isProcessed then return nil (data parts)
  • uploadTableData, archive format: isProcessed then return nil
  • uploadTableMetadataRegular: isProcessed then return processedSize (metadata/db/table.json)
  • uploadTableMetadataEmbedded, uploadSingleBackupFile, uploadBackupRelatedDir: same pattern

Only metadata.json itself is recorded unconditionally in uploadEpilogue. There is no b.dst.Walk of the remote backup when b.resume is true, so nothing reconciles the manifest with what is actually on remote.

Impact
  1. The Walk-free guarantee of #1375 is lost after any resume. downloadTableData finds zero manifest entries for every part the resumed run skipped and silently falls back to Walk (ListObjectsV2) per part, on exactly the large backups where resume is most likely to have been used.
  2. The manifest is not a trustworthy statement of what is on remote, although it is the only artifact that claims to be one. Anything that reconciles against it draws wrong conclusions. We hit this downstream: a local patch that drops a table from metadata.json when its table metadata file is absent from the manifest turned a resumed upload of a 1 TiB, 22-part table into a backup whose metadata.json had "tables": [] while every part and the table metadata were present on remote, and restore then refused it with "does not contain tables for restore".
Reproduction (directory format)
  1. clickhouse-backup create b1 on a table with several parts
  2. clickhouse-backup upload b1 and interrupt it after at least one part has finished
  3. clickhouse-backup upload --resume b1 (completes, log shows already processed for the finished parts)
  4. Download manifest.bolt.gz for b1 and list the bucket: the parts finished in step 2 are absent from the manifest, and the manifest written in step 2 has been overwritten
  5. clickhouse-backup download b1 logs a Walk fallback for those parts
Proposed fix

Make the manifest complete before it is uploaded: in uploadEpilogue, when b.resume is true, Walk the remote backup prefix once and AddFile every object (excluding the manifest itself and zero-size objects) before UploadManifest. That is one listing per resumed upload instead of one per part at every later download, and it must run before any consumer of the manifest (we initially placed it after our metadata.json reconciliation and that ordering is exactly what produced the empty tables list above). Alternative: call recordUploadedFile in each skip path, which requires the file list of every skipped part.

We run the Walk variant downstream and are happy to open a PR.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in pkg/backup/upload.go, reading uploadEpilogue and the listed upload skip paths that bypass recordUploadedFile or recordUploadedFiles. Ensure a resumed upload produces a complete manifest before UploadManifest, excluding the manifest itself and zero-size objects. Done when skipped remote objects appear in the manifest and resumed downloads no longer need per-part Walk fallbacks.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, cloud
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.