Stop undo from filling the disk - #10
Open
edaywalid wants to merge 10 commits into
Open
Conversation
The size budget lived in `undo gc`, which only runs from the shell
hook's precmd, which only fires when the command returns. A command that
runs for a day never gives it a turn, and gc skips live sessions anyway,
so the one session that is actively growing is the one nothing was ever
going to prune. A single session reached 147G and took the filesystem to
zero.
Both ceilings move into the shim, where the writes happen:
UNDO_MIN_FREE free-space floor, default 2G, checked by statvfs
amortised over every 64M of backups
UNDO_MAX_SESSION per-session cap, default 1G
Either one trips and the session stops recording and writes a `degraded`
file saying which and why. The counter is a shared mapping rather than a
static, so every process a build forks bills to the same session.
UNDO_MAX_BYTES, the per-file cap, now applies to hardlinked backups too.
It only ever covered copies, on the reasoning that a link costs nothing.
A link is not free: it is exactly what stops the original blocks being
returned when the file is unlinked, so deleting a 4G file cost 4G that
the per-file cap was supposed to have refused.
Stopping early loses undo history. Filling the filesystem takes down
everything else on the machine, and a tool that exists to save you from
mistakes should not be capable of making that one.
The default list was node_modules, .cache, __pycache__ and .git. The 147G incident was mostly .turbo/cache and .next, neither of which it covers: seg_match compares whole components, so `.turbo/cache` does not match the `.cache` pattern. These are the highest-churn and lowest-value files in a repo and a dev loop rewrites them hundreds of times an hour. Adds the tool-owned cache directories. Names a person might have picked for their own source stay out of the defaults, even though dist and target and friends hold generated output just as often, because a default that makes `rm -rf dist` unrecoverable is a worse bug than the one being fixed here. They are in examples/ignore instead. The dot-prefixed names go in a second list guarded by one strstr for "/.", so a path with no dot-directory in it still costs four component scans rather than twenty. ignored() runs on every intercepted open.
precmd wrote its done marker unconditionally. If the session directory had been removed underneath it, by gc or by someone cleaning up by hand, every subsequent prompt printed _undo_precmd:9: no such file or directory: .../sessions/17862.../done A hook has no business erroring at the prompt because its own store changed. Guard the write in all three shells. The same spot now reports a session the shim gave up on. precmd runs once per session, so the warning prints exactly once, and it is the only chance the user has to learn that the last command was not fully recorded. The hook smoke test grew a case for it. Getting it to fail without the fix took a detour worth writing down: the obvious version removed the store with a plain `rm -rf` and the store survived, because the shim was armed for that rm and backed each file up into the directory being deleted, recreating it as fast as rm unlinked it. The test disarms the shim for that one command. The underlying behaviour is real and is why gc skips live sessions.
A session the shim gave up on restores fine, it just is not the whole command. Finding that out afterwards, from files that did not come back, is the wrong way to learn it. undo list marks it with !, undo show prints the reason and that changes past that point were not recorded, and undo warns before reverting one. e2e covers both ceilings. Neither test fills a disk: the budget case uses a 1MB cap, and the floor case sets UNDO_MIN_FREE above any plausible free space so the first backup trips it.
Deleting a session whose writer is still running fails partway: rm: cannot remove '.../sessions/1786.../data': Directory not empty The shim saves each backup into the directory being deleted, so it recreates entries as fast as RemoveAll unlinks them. RemoveAll gives up having already destroyed most of the session, which is the worst of both outcomes. Reproduces reliably against a tight `rm -f` over 20k files. Rename first. That takes the whole tree out from under the shim in one step: the writer goes on appending into a directory nobody will read, and the delete that follows races with nothing. List skips a leftover .removing, which means a removal that died partway rather than a session. gc already declines to touch live sessions, so this is `undo purge --force` and anything that gets the timing wrong.
The journal descriptor and the budget mapping are __thread. A thread that exits takes the variables with it but not the descriptor and the page they point at, so a program doing its file work on short-lived threads leaked one of each per thread until it ran out of descriptors. Measured: 205 threads, 205 leaked descriptors. This is the "descriptor leak" from the incident report, though not the mechanism it proposed. The shim was not reopening the journal per intercepted call, and the suggested O_APPEND | O_CLOEXEC was already there. Eight descriptors on one journal was eight threads. A key destructor is the only thread-exit hook C offers, so the shim now links pthread. Nothing here is shared between threads, so this releases state rather than synchronising it, and the write path is untouched. The released .so still builds against glibc 2.31 on debian:11, where pthread_key_create is GLIBC_2.2.5, so the advertised 2.6 floor holds.
doctor had its own hardcoded copy of the shim's defaults, so after the list grew it told users the shim skips four things when it skips twenty-one. The list is too long to spell out now, so it reports the count and the first few. The copy is still a copy, because the list lives in C and the CLI is Go. A test parses the arrays out of shim/undo_shim.c and fails if the two drift, which is the only reason this was caught.
edaywalid
force-pushed
the
fix/session-fills-the-disk
branch
from
August 9, 2026 15:26
1c80209 to
b651c92
Compare
Review of the space guards turned up four things. The per-session cap defaulted to 1G, the same as UNDO_MAX_STORE, so one max-size session filled the entire store budget and gc dropped every other session behind it. Two numbers matching by coincidence, not a decision. It is half the store budget now, so the store holds more than one session, and raising UNDO_MAX_STORE raises it too. The statvfs interval was a __thread counter, so "one check per 64M" was per thread: a busy process checked far more often than intended and a session spread over several processes far less. It moves into the shared mapping with the byte count. UNDO_MIN_FREE and UNDO_MAX_SESSION were read with getenv on every backup, where max_bytes() beside them has always cached. They cache now, and jwrite reads the session directory once for the pair of lookups it does per journal line instead of once each. budget_stop() claimed to write the degraded marker once per session but only the shared flag enforced that, so a failed mmap meant every process wrote it. O_EXCL makes the claim true on its own.
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.
A single session grew to 147G over 23 hours and took a 320G filesystem to
zero bytes free, which broke every other program on the machine. This is
that bug and the things found next to it.
The main bug
The size budget was enforced by
undo gc, which the shell hook only callsfrom precmd, which only fires when a command returns. The command here was
an agent that ran all day and never returned, so gc never got a turn. Even
if it had,
session.GCskips live sessions and always spares the newestone, so the session that was actually growing was the one nothing was ever
going to prune.
Two ceilings now live in the shim, checked as it writes:
UNDO_MIN_FREEUNDO_MAX_SESSIONEither trips and the session stops recording and writes a
degradedfilesaying why. The counter is a shared mapping, so every process a build forks
bills to the same session budget.
0disables either one.The command itself is never blocked or slowed. undo only ever stops
recording it.
Also fixed
UNDO_MAX_BYTESdid not apply to deletions. A hardlink copies no data, sothe per-file cap skipped it, but the link is exactly what stops the
original blocks being freed when the file is unlinked. Deleting a 4G file
cost 4G the cap was supposed to refuse. This is probably where most of the
147G actually went.
.turbo/cachenever matched the
.cachepattern, because patterns match whole pathcomponents. Added the tool-owned cache directories.
thread-local, and a thread that exits takes the variable but not the
descriptor behind it, so a program doing file work on short-lived threads
climbed towards EMFILE. Measured at 205 threads, 205 leaked descriptors.
Directory not empty, having already destroyed most of it: the shimrecreated backups as fast as the delete unlinked them. Sessions are renamed
out of the way first now.
existed, so a store removed underneath it made every later prompt print
no such file or directory.undo doctorreported a hardcoded copy of the ignore list that went stalethe moment the real one grew.
Deliberately not done
dist,build,out,target,vendorandcoverageare not built-inignores, even though they hold generated output most of the time. An
accidental
rm -rf distis something people want back, and a default thatsilently made it unrecoverable would be worse than the bug being fixed.
They are in
examples/ignoreas opt-in.Live()asks "might this command still be running", and for a foregroundcommand the shell pid answers that correctly.
form would have helped here: the turbo cache files are content-hash-named,
so every path is distinct and written once, and delete backups are already
hardlinks to the original inode.
Testing
Three new e2e cases (session budget, free-space floor, thread descriptor
leak) and a hook case for the vanished store. Neither space test fills a
real disk. A unit test parses the ignore arrays out of the C source and
fails if doctor's copy drifts again.
Verified the released artifact still builds on debian:11 and keeps its
glibc 2.6 floor, since the shim now links pthread.
One judgement call worth a second opinion:
UNDO_MAX_SESSIONat 1G equalsUNDO_MAX_STORE, so one max-size session consumes the whole store budget.Not broken, since the newest session always survives gc, but the two numbers
matching is a coincidence rather than a decision.