CI only: the saucal release branch (do not merge) - #6
Open
msaggiorato wants to merge 29 commits into
Open
msaggiorato wants to merge 29 commits into
msaggiorato wants to merge 29 commits into
Conversation
A backup the kernel kills leaves nothing behind to explain itself: SIGKILL cannot be caught, so there is no error to report and no summary to print. The only account of such a run is one written while it happens. Setting RESTIC_DIAG_LOG makes restic append a line every few seconds with its own heap figures and, where a cgroup says so, the usage and limit the kernel actually decides on. Each line is flushed as it is written, so the last one says what restic was using when it died. Panics are recorded the same way before they are re-raised. Nothing is opened, sampled or written unless that variable is set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z (cherry picked from commit 032e3d9)
The restorer reserves space for each file before filling it, so that writing its blobs out of order does not fragment it. Some sandboxed filesystems, seen on managed WordPress hosting, accept that syscall and never answer it, and the consequences run deeper than a slow restore: the kernel holds the inode lock while it waits, so the file can no longer be written, shortened or unlinked — unlinking it blocks in turn, and takes its directory with it — and the waiting thread cannot be interrupted, not even by SIGKILL, so the process it belongs to can never exit. A restore stopped at the first file that needed data, left it empty, and printed nothing. Making the call is therefore an irreversible commitment, and no deadline around it can take one back: by the time the deadline passes, that file and that process are already lost. Ask the filesystem beforehand instead, with a throwaway file whose name is unlinked before the call, from a process that exists only to ask — so a filesystem that never answers strands that process instead of the restore. The answer is kept per filesystem. Whether the call succeeded is not the question, only whether it came back at all: one that fails does so promptly, and the caller tolerates a failure anyway. This is the v0.18.0 branch a fleet build is cut from; the same change sits in internal/fileio on the master-based branch. Ported from upstream/preallocation-guard commit 58b941c, rather than cherry-picked from it: upstream moved these files to internal/fileio after 0.19.1 was branched, so the same change has to sit in internal/fs here. The two differ only in the package name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
The hosts install restic from SAUCAL_RESTIC_RELEASE_BASE, which until now meant building and uploading the assets by hand. Add the script that builds them, the workflow that publishes them when a v*-saucal.* tag is pushed, and the notes for whoever changes this branch next. The script refuses a dirty working tree: Go stamps the build with the tree's state, so the same commit built with stray files around yields a different binary than one built clean, and a release has to be reproducible from its tag. Also fix this branch's own CI the way master's was fixed, so changes here can be tested: the open source MinIO server is archived and dl.minio.io answers every download with "410 Gone". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
Upstream turned the version into a constant, so build.go's -X injection no longer reaches it: on 0.19.x a stock binary and ours both report "restic 0.19.1", and an operator reading a job log cannot tell which one is running. Add the suffix that says. It also lands in the ProgramVersion recorded in each snapshot, so a repository shows which build wrote what. The VERSION file is left alone: the release asset names come from there, and the maintenance action derives the same names from its own RESTIC_VERSION. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
restic no longer hangs forever on filesystems that accept the preallocation syscall and never answer it, which is what Pressable does. Reproduction and the evidence for the approach are on repro/preallocation-hang. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
RESTIC_DIAG_LOG records what a run costs in memory as it goes, so a backup the kernel kills still says what it was using. Added for elka, which cannot be backed up on Pressable. The probe check in main stays ahead of it: a probe process exists only to ask the filesystem one question, and has no business writing diagnostics. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z # Conflicts: # cmd/restic/main.go
A change meant for upstream cannot be merged from its master-based branch into release/saucal: that would bring all of master with it, and release/saucal sits on a release tag. Cherry-pick it instead; only upstream's release tags get merged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
An integration branch cut from the release tag, merged with --no-ff, rather than the cherry-pick the previous wording described. The merge commit is where two changes touching the same place get reconciled in the open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
Deleting and republishing a release takes its download URLs offline while the rebuild runs, and anything fetching in that window gets a 404 — which happened today to someone who had been given the base URL. Fix forward with a new tag instead; rewrite history only before the first tag is pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
restic reports that an item changed, never what about it changed. rsync's --itemize-changes answers that in eleven columns, and the restorer already holds most of the comparisons it would take. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
The throwaway branch that existed for this is gone; the technique should not have to be rediscovered from upstream's red matrix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
saveDir submitted every entry of a directory and only folded the results into the directory's tree once the whole directory had been walked, so a finished entry was held -- its node, the two paths naming it and its channel, about a kilobyte in all -- until then. A directory of a million entries therefore needed roughly a gigabyte on top of the tree itself, which is enough for a host with a memory cap to kill the backup. The entries of a directory are folded into its tree in the order they appear, so a finished entry need not be kept. Fold them in as the walk goes, holding only enough to let the entries ahead of a slow one finish out of order. The file saver's queue is unbuffered, so the walk was already paced by the file readers and consuming results earlier costs no concurrency. Directories no wider than the limit behave exactly as before. Two smaller costs went with it: each entry's name is released once the paths have been built from it, and the buffer holding the tree is sized from the directory's entry count rather than being grown and copied repeatedly, which held two copies of a large tree at once. Backing up one directory of 600000 entries under a 600 MiB cap failed before and now succeeds; the same backup uncapped uses 39% less memory at a million entries. The tree written is byte for byte unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
From saucal/wide-directory-memory, cut from v0.19.1 and cherry-picked from upstream/bound-wide-directory-memory so the fleet build and the upstream diff stay the same change. A directory with very many entries cost about a kilobyte of memory per entry, because saveDir held every finished entry until the whole directory had been walked. That is what makes elka -- one uploads directory with hundreds of thousands of files -- unbackupable under Pressable's memory watchdog, which kills restic with SIGTERM at roughly 680 MB. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
SAUCAL.md still claimed this branch carried one change, from before the diagnostics were merged. List all three, and record the measured memory per directory entry along with the limit that no tuning gets past, so the next person does not have to rediscover where the wall is. Also ignore /.worktrees/, which is where this work was done. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
verifyCiphertext decrypted and decompressed the blob it had just sealed and rehashed it, holding the whole plaintext a second time to do so. For the tree of a very wide directory -- several hundred megabytes -- that doubled the cost of saving it at exactly the moment the first copy was still live. Hash the plaintext as it decompresses instead, for blobs large enough that setting up a streaming decoder is worth it. What gets verified is unchanged; only the copy is gone. Measured on a backup of one directory of a million entries, peak RSS falls from 773 MB to 603 MB. Disabling verification entirely reaches 592 MB, so almost all of the difference was the copy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
A directory's tree is one blob listing every entry, a few hundred bytes each, and it was built in memory: held while the directory was walked, then compressed and encrypted from that same copy. For a directory of a million entries that is several hundred megabytes, and it was by then the largest single cost of the backup -- enough that a host which caps process memory kills the backup outright. Write the tree of a wide directory to a temp file as the walk goes, and hand the repository the file rather than a buffer. saveBlobFromReader hashes and compresses in one pass as it reads, declaring the plaintext size in the zstd frame header exactly as EncodeAll does, so whoever reads the blob back can still size its buffer in one go. The plaintext never exists in one piece anywhere. The temp file comes from fs.TempFile, which returns a file already unlinked, so closing it is the whole of the cleanup. Creating it is a nicety rather than a requirement: if it fails, the tree is built in memory as before. Directories below the threshold are untouched. This is what rsync does with its file list -- stream the metadata rather than materialise it -- adapted to a format that needs each directory's listing addressed as one blob. One directory of a million entries: peak RSS 208 MB, against 1248 MB before any of this work and 603 MB after bounding the pending nodes. 1.2M entries succeeds under a 400 MiB cap. The tree written is byte for byte unchanged, a restore of it matches the source exactly, and check --read-data passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
Two more commits from saucal/wide-directory-memory, cherry-picked from upstream/bound-wide-directory-memory: verification no longer holds a second copy of a large blob, and the tree of a directory of 50000 entries or more is written to a temp file and streamed into the repository instead of being held in memory. Together with the earlier bounding of pending nodes, one directory of a million entries now peaks at 208 MB rather than 1248 MB and succeeds under a 250 MiB cap, which is what elka's uploads/2021/06 needs to be backed up at all on Pressable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
…eeds The numbers in this file were from before the tree was streamed. Replace them, say which of the three changes saved what, and note the new operational dependency: the temp file holding a wide directory's tree is a few hundred megabytes, so a host whose /tmp is small or memory-backed needs TMPDIR pointed at real disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
The file named pull request #1, which has been closed for a while, so the branch the fleet installs was getting no test matrix at all — pushes to anything but master are ignored by the workflow. Name the replacement and say what goes wrong if it is closed too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
…eat it A pull request that conflicts with its base gets no pull_request workflow runs at all, because GitHub cannot build a merge commit for it. Based on master, the CI pull request for this branch therefore showed only size-label and looked healthy while running nothing -- which is how three patches landed here without the test matrix ever having run. Park a base at v0.19.1 and target that instead. Also record that the release workflow's Test step covers three packages that have nothing to do with the archiver or the repository, so it never substituted for the matrix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
The temp file holding a wide directory's tree was only closed once the tree had been handed to the repository, so four paths leaked the descriptor: either error return in treeSaver.save, a cancellation that made Save drop the job before a worker took it, and any error out of saveDir once the builder existed. os.File's finalizer closes an abandoned descriptor eventually, but an error affecting many wide directories could exhaust the table before the garbage collector got to them. Release in a defer as soon as ownership is known, hand ownership over explicitly where it passes to the tree saver, and make release safe to call twice and on a builder that never spilled. Also add the corruption tests the streaming verification was missing. It is the last check before a blob is uploaded, so a version of it that always passed would be worse than no check at all: prove it rejects a wrong id, a bit flipped at three different offsets, a truncated stream and input that was never compressed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
From saucal/wide-directory-memory. Four paths leaked the descriptor of the temp file holding a wide directory's tree -- both error returns in treeSaver.save, a cancellation that dropped the job, and any error out of saveDir once the builder existed. Also adds the corruption tests the streaming verification was missing, since it is the last check before a blob is uploaded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
A cancelled backup returns from treeSaver.save while the repository is still reading the temp file holding a wide directory's tree, so releasing that file in a defer there closed it underneath the upload, which then failed with "file already closed". The failure was confined to a path that was already failing, and the size check in saveBlobFromReader means no short blob could result, but the error was one we invented rather than one reported by anything real. Hold the file until the repository is finished with it, which the callback signals, and keep the defer for the paths where it was never handed over. TestTreeSaverSpilledTreeSurvivesCancel reproduces it: it reaches the spilling path by asking for a directory as wide as the threshold rather than by feeding the builder that many nodes, and holds the mock repository's read until after the cancellation, so the ordering is deterministic rather than raced. Without this change it fails with exactly the error above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
saveBlob hashes a blob, asks the index whether the repository already has
it, and compresses only if it does not. saveBlobFromReader could not follow
that order, because it hashed and compressed in a single pass, so the tree
of an unchanged wide directory was compressed on every backup and then
thrown away when the index said the blob was known.
Read the stream twice instead: once to hash, and again to compress only
when the blob turns out to be new. That restores saveBlob's order, and
costs a second pass over a temp file whose pages the kernel has just
written and still holds.
Measured on a 47 MB tree shaped like a real WordPress uploads directory --
varied names and times, and a content id per node, so it compresses like
the real thing rather than like a repeated string:
hash only 43 ms
compress only 230 ms
hash + compress 275 ms
So recognising a known tree cost six times what it needed to. On the
committed benchmark, whose tree is more repetitive and therefore kinder to
the compressor, saving a known blob goes from 33 ms to 15 ms and saving a
new one from 70 ms to 59 ms. For a directory of a million entries this is
seconds per backup rather than a bounded cost, since the tree grows with
the directory.
The stream therefore has to be seekable, which the interface now says. Its
one caller hands over a temp file. Also stop asking the encoder to close a
stream it has already closed, and size the compressed buffer at a quarter
of the plaintext rather than an eighth, which is nearer what a real tree
achieves and so avoids a resize that would hold two buffers at once.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
Two fixes from saucal/wide-directory-memory, each with a test that fails without it: - A cancelled backup closed the temp file holding a wide directory's tree while the repository was still reading it, inventing a "file already closed" error on an already failing path. - The streaming path compressed in order to hash, so the tree of an unchanged wide directory was compressed on every backup and then thrown away when the index reported the blob known. Hashing first restores the order saveBlob uses; on a realistic 47 MB tree, hashing costs 43 ms against 275 ms to hash and compress. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
golangci-lint's revive rule wants context.Context as a function's first parameter, and the benchmark helper added with the previous commit had it third. Caught by the lint job rather than locally; golangci-lint v2.12.2 is now installed here to check before pushing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
… leaves The temp file holding a wide directory's tree was the one thing about this build nobody could check without a shell on the host. elka's backup on 2026-09-18 settles it: the action unpacks into /tmp/issh.XXX and TMPDIR is the same filesystem, so there is room and nothing to configure. Also note what testing a memory limit runs into: a killed backup cannot release its own lock, and a lock from another host counts as stale only after 30 minutes, so a retry inside that window is refused with "restic check failed". That cost a run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
The memory-ceiling idea was written before the diagnostics had said where the memory went. They have: directory width, not index size, and that is fixed and measured on the real host. Rewrite it around what is actually left, which is setting GOMEMLIMIT. Add the two things this work surfaced and did not do. Reading a wide directory's tree still materialises the whole blob, so elka's backup now fits in 325 MB while its restore would not -- that asymmetry is the larger half of the problem now. And the entry names are the one term still scaling with width, which rsync's slab-and-offsets approach would cut by about a fifth, worth doing only if a site turns up that needs it. Also record the state of the upstream series: six commits, placeholder changelog numbers, and the argument for splitting it across two pull requests rather than one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z
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
1 participant
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.
Exists only to run the full test matrix against
release/saucal, the branch the maintenance action installs on WordPress hosts. The workflow runs on pull requests, so without an open PR this branch gets no CI at all — the old PR that did this was closed, and SAUCAL.md still pointed at it.Do not merge.
mastertracks upstream and is never committed to;release/saucalis an upstream release tag plus our patches.🤖 Generated with Claude Code
https://claude.ai/code/session_017d5y4KNmm2N5ySPPH1H49Z