backup: add verify command - #1337
Merged
Merged
Conversation
bigbes
force-pushed
the
bigbes/tntp-8670-backup-verify
branch
3 times, most recently
from
July 30, 2026 06:13
3560f19 to
e5385aa
Compare
Base automatically changed from
g.rybakov/tntp-8608-start-rework-tests
to
master
July 30, 2026 08:56
bigbes
force-pushed
the
bigbes/tntp-8670-backup-verify
branch
2 times, most recently
from
July 30, 2026 12:49
c145fc0 to
b6d7a38
Compare
bigbes
force-pushed
the
bigbes/tntp-8670-backup-verify
branch
3 times, most recently
from
July 30, 2026 19:56
6ff9419 to
7efa1bb
Compare
sssciel
approved these changes
Jul 30, 2026
Opening a filesystem storage walked its whole root and removed every leftover .tt-backup-* file older than a day. Only Put creates those files, but every caller paid for the sweep - including the ones that promise to change nothing at all. `tt backup verify` is the first of those, and the promise is explicit: "Nothing is ever deleted or repaired". It was untrue for a file storage before verify had read a single byte, and what it removed was the residue of an interrupted upload - the very thing an operator running verify may be trying to look at. The sweep now runs on the first Put instead, so writers still clean up after writers and a reader deletes nothing. The existing read-only assertions could not have caught this: they drive an in-memory store and never construct an fs.Storage. Part of TNTP-8608
Validation required the shards map to be present but not to hold anything, so a manifest with an empty one passed as healthy while referring to no archive at all. Nothing downstream expects that. `tt backup verify` reported the manifest as fine and its archive as unreferenced, which reads as "delete this"; `tt backup gc` would then collect the live archive of a backup that still exists once it aged past --orphan-age. A backup of no shards is not a backup, so it is invalid. Empty and nil stay separate errors: one is a hand-edited manifest, the other a missing field, and an operator reading the report wants to know which. Part of TNTP-8608
bigbes
force-pushed
the
bigbes/tntp-8670-backup-verify
branch
from
July 31, 2026 06:19
7efa1bb to
cb7b1a4
Compare
patapenka-alexey
approved these changes
Jul 31, 2026
bigbes
force-pushed
the
bigbes/tntp-8670-backup-verify
branch
from
July 31, 2026 08:51
4ccf696 to
7857221
Compare
Add `tt backup verify`, a read-only health check of a backup storage. It loads the manifest chain, recomputes sha256 of every referenced archive straight from the GET stream (archives are never spooled to disk), and lists archives no manifest refers to. Reported problem classes: unreadable and structurally invalid manifests, missing and unreadable archives, checksum mismatches, breaks in the previous_backup_id chain (orphans and forks), increments whose vclock_begin does not continue their ancestor, and dangling archives. Chain problems come from cli/backup/chain, inherited ones included. Split chain.Load into Load and LoadPartial: a diagnostic caller must report a single unreadable manifest and keep checking the rest of the storage, the way `tt cluster topology` degrades per instance, while a caller that recovers from the chain still fails loud on a missing link. LoadPartial degrades on two more defects that used to abort the whole run: a manifest with no backup_id and two objects claiming the same one. Neither can become a chain entry, so both are reported against their storage key - the only name they have left, now carried on the issue itself rather than buried in an error string. Nothing is ever written or deleted: removing backups stays with `tt backup gc`. The command exits with 2 when problems are found, so a cron job tells an unhealthy storage from a run that could not check anything at all (exit code 1); 2 matches the fail-loud code of `tt backup start`. A cancelled context or an expired --timeout therefore aborts rather than reporting the archives it never read as unreadable, which would let a deadline masquerade as corruption; and --timeout 0 means no limit rather than "already expired". An archive whose manifest could not be read is not called dangling: the manifest does claim it, it just cannot be parsed to say so, and gc acts on exactly this issue. Chain problems are attached in backup id order, so a manifest inheriting from several broken ancestors reports them the same way every run and a cron consumer diffing --format json sees only real changes. A previous_backup_id cycle is reported instead of dropping the manifests involved out of the chain entirely. Closes TNTP-8670
backup_helpers built the local artifact directory from
realpath("/tmp"), while cli/backup builds it from Go's os.TempDir().
The two agree on Linux, where TMPDIR is normally unset, but on macOS
os.TempDir() returns the per-user $TMPDIR under /var/folders, so the
helper pointed at a directory tt never wrote to.
Two tests failed because of it and neither failure had anything to do
with the code under test: test_backup_start compared the archive path
printed by 'tt backup start' against the wrong parent directory, and
test_finalize failed a step earlier still, creating a neighbouring
artifact in a directory that did not exist.
tempfile.gettempdir() honours TMPDIR the same way os.TempDir() does, so
both sides land in the same directory on either platform.
Part of TNTP-8608
`tt backup upload` writes every archive to data/ and the manifest that references them last, so that a half-uploaded backup is not a backup. Between the first archive and the manifest - the whole upload, hours on a large cluster - those archives are unreferenced by construction, and verify reported each of them as a dangling archive and exited 2. A cron verify overlapping the cron backup therefore raised the storage-unhealthy alert on a cluster doing exactly what it should. Split the two apart the way gc already does: an unreferenced archive of a backup newer than every stored manifest, or any unreferenced archive when the storage holds no manifest at all, is an upload in progress. It is still reported, because an operator wants to see everything that is in the storage, but it is marked informational and does not make the storage unhealthy. An archive whose backup id the pipeline has already moved past is abandoned as before, and still fails the check. Findings carry an `informational` flag for that, and Report.Problems counts the rest, so a JSON consumer can filter and the table output does not dress an upload up as a defect. Part of TNTP-8670
bigbes
force-pushed
the
bigbes/tntp-8670-backup-verify
branch
from
July 31, 2026 10:25
7857221 to
a5499b2
Compare
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
3 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.
Add
tt backup verify, a read-only health check of a backup storage. It loads the manifest chain, recomputes sha256 of every referenced archive straight from the GET stream (archives are never spooled to disk), and lists archives no manifest refers to.Reported problem classes: unreadable and structurally invalid manifests, missing and unreadable archives, checksum mismatches, breaks in the
previous_backup_idchain (orphans and forks), increments whosevclock_begindoes not continue their ancestor, and dangling archives. Chain problems come fromcli/backup/chain, inherited ones included.chain.Loadis split intoLoadandLoadPartial: a diagnostic caller must report a single unreadable manifest and keep checking the rest of the storage, the waytt cluster topologydegrades per instance, while a caller that recovers from the chain still fails loud on a missing link.Nothing is ever written or deleted: removing backups stays with
tt backup gc. The command exits with 2 when problems are found, so a cron job tells an unhealthy storage from a run that could not check anything at all (exit code 1); 2 matches the fail-loud code oftt backup start.Two prerequisite fixes come first in the branch, each independently reviewable and each a defect in code already on master rather than anything verify introduces.
storage/fsswept stale.tt-backup-*files when the storage was opened, which made verify's own promise ("Nothing is ever deleted or repaired") untrue before it had read a byte; the sweep moved to the firstPut, so writers still clean up after writers and readers delete nothing. And a manifest with an emptyshardsmap validated as healthy while referring to no archive at all, so verify called it fine and its archive unreferenced - which reads as "delete this".Closes TNTP-8670