Skip to content

libstore: re-check free space after auto-GC acquires the GC lock - #14

Merged
ethancedwards8 merged 1 commit into
mainfrom
devin/1789582389-autogc-recheck-after-lock
Sep 16, 2026
Merged

ethancedwards8 merged 1 commit into
mainfrom
devin/1789582389-autogc-recheck-after-lock

Conversation

@schew2381

Copy link
Copy Markdown

Motivation

On a busy CI runner, min-free auto-GC produces a thundering herd: every daemon worker process independently notices avail < min-free, and each one queues for the exclusive gc.lock to run its own full GC pass. The first pass reclaims the space, but the queued passes still run — collectGarbage never re-checks disk after acquiring the lock — so a 700k-path store stays wedged for hours while N-1 redundant full passes drain (each holding the exclusive lock and blocking every build/copy on the host). This is what took down nix-runner-x86-1 today: ~70 queued passes, 6+ hours after free space was already back above max-free.

The gcRunning guard in autoGC does not help: it is per-process, and every daemon worker is its own process.

Context

Fix: let auto-GC ask collectGarbage to re-check free space after it holds the lock, and return without collecting if space is already fine. The first waiter does the real pass; every other one takes the lock for milliseconds and moves on. min-free stays useful as a safety net with no herd.

 struct GCOptions {
     uint64_t maxFreed{...};
+    std::optional<uint64_t> skipIfAvailAbove;   // internal, not on the daemon wire
 };

 void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) {
     FdLock gcLock(fdGCLock.get(), ltWrite, true, "waiting for the big garbage collector lock...");
+    if (options.skipIfAvailAbove && getStoreAvail() >= *options.skipIfAvailAbove) {
+        printInfo("skipping auto-GC: %d bytes free after acquiring the GC lock (threshold %d)", ...);
+        return;
+    }

 void LocalStore::autoGC(bool sync) {
     ...
     options.maxFreed = gcSettings.maxFree - avail;
+    options.skipIfAvailAbove = gcSettings.minFree;
  • The free-space lambda in autoGC is factored into LocalStore::getStoreAvail() so collectGarbage can reuse it (same _NIX_TEST_FREE_SPACE_FILE override and statvfs path).
  • Only auto-GC sets the option; explicit nix-store --gc / nix store gc are unchanged.
  • Not serialized over the daemon protocol (daemon.cc / remote-store.cc untouched), so no wire-format change.

Test: tests/functional/gc-auto-skip.sh — process A holds the GC lock (via the _NIX_TEST_GC_SYNC_2 fifo), process B's build triggers auto-GC and queues behind it, fake free space is raised above min-free before A releases; asserts B logs skipping auto-GC and none of the garbage paths were deleted. gc-auto.sh and gc-non-blocking.sh still pass. Ran inside nix develop .#native-clangStdenv (the default gcc devshell has a preexisting -Wchanges-meaning error in closure.hh, unrelated to this change).

Companion operational fix (already merged): exa-labs/monorepo#146910 reaps orphaned nix-daemon --stdio sessions. Rolling this out requires bumping the nix-src rev pinned in the Colmena flake.

Link to Devin session: https://app.devin.ai/sessions/cb6b9562567c47a898ccb1ed0a050c9a
Open in Devin Desktop: https://app.devin.ai/desktop/session/cb6b9562567c47a898ccb1ed0a050c9a?variant=devin
Requested by: @schew2381

autoGC computed maxFreed before blocking on the exclusive gc.lock, and
collectGarbage never re-checked free space once it won the lock. The
gcRunning guard is per-process, so each nix-daemon worker that observed
low disk queued its own full GC pass behind the lock — the "GC herd":
after the first pass reclaimed the space, N more passes still ran.

Add GCOptions::skipIfAvailAbove: when set, collectGarbage re-checks
filesystem free space right after acquiring gc.lock and returns early
if enough is free. autoGC sets it to minFree so queued passes skip
themselves once an earlier collector has done the work. getAvail is
factored into LocalStore::getStoreAvail() for reuse.

Assisted-by: devin:claude
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@ethancedwards8
ethancedwards8 merged commit bff59c3 into main Sep 16, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants