Artifact 59e75a6fefe9fdb951bc6bd1e3479b2fa557f80eabb65e8ea73d060d76ce2092:
- File
src/cmd/verify.zig
— part of check-in
[b224ffeb40]
at
2026-07-31 21:12:22
on branch logstore-slice2
— logstore: openReadOnly scans header-only; verified_on_open is gone
The speedup reaches production. openReadOnly is the path every CLI verb takes -- Repository.open falls back to it, and Repository.logStore, the content channel behind cat, artifact, checkout and sync, calls it with no cache in front. It scanned every payload byte on every invocation:
10k frames 42 MB 42.2ms -> 10.3ms 10k frames 410 MB 298.0ms -> 12.3ms 100k frames 416 MB 511.9ms -> 115.4ms 100 frames 419 MB 321.4ms -> 0.10ms
Open cost is now a function of frame count, not repository size.
verified_on_open was the flag openReadOnly set to tell readFrameAt that the scan had already checked every frame, so reads could skip their own crc. With no scan doing that, the exemption had nothing left to rest on and the flag is deleted. readFrameAt now crc-checks unconditionally: it is the only thing standing between a rotted payload and a caller that believes it, and restoring any skip there means restoring the scan.
Detection is relocated, not removed. A payload bitflip no longer voids the open -- the rest of the repository stays usable -- and surfaces the first time anyone reads those bytes: "zit cat" and "zit artifact" both refuse with 'repository is corrupt', and "zit verify" names the artifact and the offset. That verb shipped first for exactly this reason.
openForVerify is deleted. It existed because openReadOnly both aborted on a damaged store and granted read-time trust, neither of which verify could accept; both properties are gone, it became byte-identical to openReadOnly, and its own doc comment said to remove it in this commit.
The DST prove-it-can-fail arm is inverted rather than deleted. It asserted that openReadOnly WOULD serve latent rot silently, across a seed range where at least one corpus lands a payload rot on a raw frame whose header survived. That hazard is gone, so the same range now asserts the opposite: the read path never serves rot as success. trust_on_read goes with it -- there are no longer two paths to select between. Losing the eager whole-log scan cost nothing here, because per-read verification is what was catching this all along.
Net -40 lines.
Verified: fmt clean, test 2327/2327, gate, e2e 671/671, dst 6 seeds x 1000 steps. The Slice 1 tripwire -- the assertion that openReadOnly still rejected eagerly, written to fail if the flag was deleted without an fsck verb -- fired on this commit, which is the one it was aimed at. (user: dmestas, size: 17838) [annotate] [blame] [check-ins using] [more...]
//! `zit verify` — this repository's own fsck. //! //! NOT AN RFC-MANDATED VERB. repo-state §7.3 blesses a full rebuild from the //! artifacts as a recovery fallback, but it specifies nothing about a //! verification command; the shape of this one is zit's own design decision and //! no spec citation is claimed for it. //! //! It exists because the open-time scan is no longer where damage is noticed. //! The scan used to stream every payload byte through the crc on every open, //! which made corruption detection a side effect of paying Θ(repository bytes) //! to open. That cost is being deleted; this verb is the on-demand replacement, //! the same split git draws between ordinary operation and `git fsck`. //! //! TWO PASSES, DELIBERATELY LAYERED, because they fail independently: //! //! 1. The whole-log frame pass (`logstore.FrameVerifier`) reads every frame's //! bytes and checks the whole-frame checksum. It is built directly on the //! store FILE, so it works on a store no `open` would accept — which is the //! entire reason an fsck exists. It sees config records and shun rows, //! which the second pass structurally cannot. //! 2. The per-artifact pass reconstructs every stored artifact through its //! delta chain and re-hashes it against its own name. It catches a frame //! whose checksum is perfectly valid over the wrong bytes — a fault the //! first pass is blind to by construction. //! //! Scope stops at content artifacts: manifests are not re-parsed. A manifest //! whose bytes hash to its name is byte-identical to what `zit commit` wrote and //! what sync validated on ingest, so re-parsing it would catch a zit bug rather //! than storage rot. `verify_help` says so. const std = @import("std"); const cli = @import("../cli.zig"); const logstore = @import("zit").logstore; const storage = @import("zit").storage; const artifact = @import("zit").artifact; // Shared CLI helpers that live in main.zig, aliased so this file reads like its // siblings in src/cmd/. const main = @import("../main.zig"); const repo_filename = main.repo_filename; const reasonOf = main.reasonOf; const stateOom = main.stateOom; const not_a_repo_msg = main.not_a_repo_msg; const repoReadFail = main.repoReadFail; const digestToWireName = main.digestToWireName; const verify_help = main.verify_help; // The two named-length couplings this file's output depends on, tied at // compile time rather than by coincidence: a frame's hash is at most // `max_hash_len` bytes and `digestToWireName` asserts its input fits // `digest_len`, so `name_buf` below is only the right size while the two agree. comptime { std.debug.assert(main.digest_len == logstore.max_hash_len); } /// Running tallies across both passes, so the verdict can state the byte /// accounting as a separate fact from the fault count — and can state WHICH /// pass found what. The split is not cosmetic: one physical bitflip in a /// content frame is found by both passes (a bad checksum, then an artifact that /// will not reconstruct), and a single aggregate "2 faults" would read as two /// damaged frames. const Tally = struct { frame_faults: u64 = 0, artifact_faults: u64 = 0, /// Artifacts the per-artifact pass reconstructed, or null when that pass /// did not run. Null rather than zero on purpose: "0 artifacts" would read /// as "this store holds none", which is a different claim from "nobody /// looked". artifacts: ?u64 = null, /// The per-artifact pass could not open the store at all. Its own kind of /// fault, and one that has to raise the exit code: a repository every other /// verb would refuse to open is not a clean repository, and `verify_help`'s /// exit table promises 1 when "the store could not be read". unreadable: bool = false, fn faults(self: Tally) u64 { return self.frame_faults + self.artifact_faults + @intFromBool(self.unreadable); } }; /// Checks this repository's store for damage. Read-only in the strongest sense: /// it takes no lock, opens nothing for write, and never truncates — so it is /// safe to run against a store a `zit serve` has open. /// /// Findings stream to stderr in walk order as they are found; the clean summary /// is the only thing that ever reaches stdout, so a faulty run leaves stdout /// empty and nothing downstream mistakes damage for a report. pub fn runVerify( gpa: std.mem.Allocator, io: std.Io, arena: std.mem.Allocator, verb_args: []const []const u8, streams: cli.Streams, amb: cli.Ambient, ) cli.Exit { const help = switch (cli.parseNoPositionalArgs(verb_args)) { .ok => |a| a.help, .unknown_option => |tok| return cli.usageError(streams, "verify", .unknown_option, tok), .extra_argument => |tok| return cli.usageError(streams, "verify", .extra_argument, tok), }; if (help) { streams.out.writeAll(verify_help) catch {}; return .ok; } // `-R` is a global option parsed before dispatch, so it never reaches the // verb's own parser; `resolveRoot` is where the two routes converge. const root = (cli.resolveRoot(arena, io, amb.repo_override) catch return stateOom(streams, "verify")) orelse { streams.err.writeAll(not_a_repo_msg) catch {}; return .failure; }; var root_dir = root.dir; defer root_dir.close(io); var file = root_dir.openFile(io, repo_filename, .{}) catch return repoReadFail(streams, "verify"); defer file.close(io); const st = file.stat(io) catch return repoReadFail(streams, "verify"); var fs = storage.FileStorage.init(io, file, st.size); var tally: Tally = .{}; // --- pass 1: every frame's whole-frame checksum ------------------------ var v = logstore.FrameVerifier.init(fs.storageFile()) catch |e| { // The FILE header itself. Nothing past it can be read as frames at all, // so there is no inventory to take and no second pass to attempt. streams.err.print("zit verify: the store's file header is unreadable: {s}\n", .{reasonOf(e)}) catch {}; streams.err.writeAll(advisory) catch {}; return .failure; }; var fatal = false; // STATED LIMIT ON OUTPUT: one stderr line per damaged frame, bounded by the // store's frame count and streamed as it is found. Deliberately uncapped — // the inventory IS the product here, and an operator who wants less can // pipe it; a cap would make the verb unable to answer the question it // exists to answer. while (v.next()) |fault| { tally.frame_faults += 1; renderFrameFault(streams, fault); if (fault.fatal()) { fatal = true; break; } } const totals = v.totals(); if (!totals.complete and !fatal) { // DEFENCE AGAINST THE WORST OUTPUT THIS VERB CAN PRODUCE. Today the // only way the walk ends short is a fatal fault, which is already // reported and already raises the exit code — so this branch is // unreachable. It exists because the coupling is invisible from here: // if a future change ever lets the walk stop early without saying why, // a short walk must be a fault rather than a silent "0 faults" over an // unexamined store. tally.frame_faults += 1; streams.err.print( "zit verify: the frame walk ended at offset {d} without reporting why\n", .{totals.bytes_checked}, ) catch {}; } if (totals.torn_tail_at) |at| { // A NOTE, NOT A FAULT, and it must not move the exit code: `open` // recovers a torn tail silently because it means an interrupted write. // Sending an operator to re-clone over an ordinary crash would be a // worse failure than the one being reported. streams.err.print( "zit verify: incomplete final frame at offset {d} from an interrupted write; the next write truncates it, this is not corruption\n", .{at}, ) catch {}; } // --- pass 2: every artifact reconstructed and re-hashed ----------------- if (fatal) { streams.err.writeAll("zit verify: per-artifact verification skipped: the frame walk could not continue\n") catch {}; } else if (totals.torn_tail_at != null) { // `openReadOnly` never truncates, so a torn tail is `error.Corrupt` // to it. Say which of the two passes did not run rather than reporting // a clean second pass that never happened. streams.err.writeAll("zit verify: per-artifact verification skipped: the store has an incomplete final frame\n") catch {}; } else { verifyArtifacts(gpa, streams, fs.storageFile(), &tally); } // --- verdict ------------------------------------------------------------ if (tally.faults() == 0) { if (tally.artifacts) |n| { streams.out.print( "verified: {d} {s}, {d} {s}, {d} bytes, 0 faults\n", .{ totals.frames, plural(totals.frames, "frame", "frames"), n, plural(n, "artifact", "artifacts"), totals.bytes_checked }, ) catch {}; } else { // The per-artifact pass did not run; stderr already said why. The // summary must not imply it did. streams.out.print( "verified: {d} {s}, {d} bytes, 0 faults; per-artifact verification did not run\n", .{ totals.frames, plural(totals.frames, "frame", "frames"), totals.bytes_checked }, ) catch {}; } return .ok; } // Each pass's count is stated on its own, because one physical bitflip in a // content frame produces a finding from BOTH — and "2 faults" would read as // two damaged frames. "N of M bytes checked" is mandatory and is a SEPARATE // FACT again: a verb that reports one fault after stopping at 8% of the // store has lied about the other 92%. if (tally.artifacts != null) { streams.err.print( "zit verify: {d} frame {s}, {d} artifact {s}, {d} of {d} bytes checked\n", .{ tally.frame_faults, plural(tally.frame_faults, "fault", "faults"), tally.artifact_faults, plural(tally.artifact_faults, "fault", "faults"), totals.bytes_checked, totals.total, }, ) catch {}; } else { streams.err.print( "zit verify: {d} frame {s}, {d} of {d} bytes checked; per-artifact verification did not run\n", .{ tally.frame_faults, plural(tally.frame_faults, "fault", "faults"), totals.bytes_checked, totals.total }, ) catch {}; } streams.err.writeAll(advisory) catch {}; return .failure; } /// One unconditional closing line, never branched on shun state: an operator /// reading a damage report needs the repair route stated the same way every /// time, and a conditional hint is one more thing to be wrong about. const advisory = "zit verify: cloning from a healthy peer is the reliable repair; 'zit pack' drops artifacts already shunned\n"; fn plural(n: u64, one: []const u8, many: []const u8) []const u8 { return if (n == 1) one else many; } /// Renders one frame-pass finding. THE FULL LOWERCASE HEX NAME, never /// abbreviated: the line's job is to be pasted into `zit artifact` or /// `zit shun`, and a truncated name cannot be. /// /// The name is available on a payload fault at all because `readFrameHeader` /// checksum-verifies the prefix and header windows SEPARATELY from the payload, /// and the hash lives in the header window — so a payload bitflip leaves the /// name intact. That is what makes damage nameable from the frame pass alone. fn renderFrameFault(streams: cli.Streams, fault: logstore.FrameFault) void { const kind = fault.kind orelse { const why = switch (fault.cause) { error.Corrupt => "header checksum failed", error.Storage => "input/output error", }; streams.err.print( "zit verify: frame at offset {d}: {s}; cannot verify past this point\n", .{ fault.offset, why }, ) catch {}; return; }; var name_buf: [2 * logstore.max_hash_len]u8 = undefined; const name = digestToWireName(fault.name(), &name_buf); // A post-header fault is one of two things, and they are not the same // advice: damaged bytes on a working disk, or a disk that would not hand // the bytes over at all. The walk continues in both cases. const what = switch (fault.cause) { error.Corrupt => "payload checksum mismatch", error.Storage => "payload bytes could not be read", }; switch (kind) { .config => streams.err.print( "zit verify: frame at offset {d} (config record): {s}\n", .{ fault.offset, what }, ) catch {}, .shun => streams.err.print( "zit verify: frame at offset {d} (shun row for {s}): {s}\n", .{ fault.offset, name, what }, ) catch {}, else => streams.err.print( "zit verify: frame at offset {d} ({s}, {s}): {s}\n", .{ fault.offset, name, kindLabel(kind), what }, ) catch {}, } } /// A frame's payload encoding in operator words rather than field names. fn kindLabel(kind: logstore.FrameKind) []const u8 { return switch (kind) { .raw => "full content", .zlib => "compressed content", .delta => "delta", .config => "config record", .shun => "shun row", _ => "unknown frame kind", }; } /// Pass 2. Opens the store through `openReadOnly`, which now scans /// `.headers_only` and grants no read-time trust — so it neither aborts on the /// damaged store being fsck-ed nor lets a frame read below skip its own /// checksum. Both properties used to require a separate `openForVerify`; that /// function became byte-identical to this one and was deleted. fn verifyArtifacts( gpa: std.mem.Allocator, streams: cli.Streams, file: storage.StorageFile, tally: *Tally, ) void { // A FAULT, NOT A SKIP. The frame pass walks raw bytes and can therefore // come up clean on a store `scan` still rejects — a checksum-valid but // malformed shun row is exactly that shape. Reporting "0 faults" and // exiting 0 on a repository every other verb refuses to open is the one // answer this verb must never give, and `verify_help`'s exit table already // promises 1 when "the store could not be read". var store = logstore.LogStore.openReadOnly(gpa, file) catch |e| { tally.unreadable = true; streams.err.print("zit verify: the store could not be opened for per-artifact verification: {s}\n", .{reasonOf(e)}) catch {}; return; }; defer store.deinit(gpa); const rids = store.ridsInPersistedOrder(gpa) catch { tally.unreadable = true; streams.err.writeAll("zit verify: the store could not be inventoried for per-artifact verification: out of memory\n") catch {}; return; }; defer gpa.free(rids); // From here the pass HAS run, so the count is a real zero rather than // "nobody looked" even if every rid turns out to be a phantom. tally.artifacts = 0; var cache = logstore.PlaintextCache.init(); defer cache.deinit(gpa); for (rids) |rid| { var name_buf: [2 * logstore.max_hash_len]u8 = undefined; const name = digestToWireName(store.hashOf(rid), &name_buf); // A shunned artifact whose bytes are still on disk IS verified and IS // reported. `zit pack` filters before verifying so corruption cannot // block its own removal; verify has no removal to block, and must not // claim clean about bytes it declined to look at. // `isShunned` asserts a COMPLETE digest width — the shun table is keyed // by complete artifact name and cannot answer about a prefix. A store // holding a frame of some other width is not something to trap on // inside an fsck, so the width is checked rather than assumed. const shunned = logstore.isCompleteDigest(store.hashOf(rid).len) and store.isShunned(store.hashOf(rid)); // `contentUnmasked`, not `content`: the masked reader answers null for a // shunned artifact, so a masked loop would report "0 faults" about bytes // it never read. const content = store.contentUnmasked(gpa, rid, &cache) catch |e| { // A read error is a FINDING, not an abort — the next artifact may // be fine, and an fsck that stopped here would under-report. tally.artifact_faults += 1; streams.err.print( "zit verify: artifact {s}{s}: cannot reconstruct: {s}\n", .{ name, if (shunned) " (shunned, not yet reclaimed)" else "", reasonOf(e) }, ) catch {}; continue; } orelse continue; // unreachable: `ridsInPersistedOrder` already drops phantoms defer gpa.free(content); tally.artifacts = tally.artifacts.? + 1; // `artifact.verify` is total — a name of any length it does not // recognise is `false`, never a trap — so unlike `isShunned` above it // needs no width guard. It does mean a store carrying some hash width // other than 20 or 32 bytes would be reported as a name mismatch; // nothing zit writes produces one, and a store that holds one is // damaged by any reading. if (!artifact.verify(name, content)) { tally.artifact_faults += 1; streams.err.print( "zit verify: artifact {s}{s}: content does not hash to its name\n", .{ name, if (shunned) " (shunned, not yet reclaimed)" else "" }, ) catch {}; } } }