Repository navigation
Make idle pantsd lease extension cheap - #23739
Conversation
pantsd's `StoreGCService` re-leases every digest its graph references every `LOCAL_STORE_LEASE_TIME_SECS / 100` (72s), and each pass wrote a new expiry for every entry: one LMDB write transaction per digest, or an `mtime` update per large file, whether or not anything had run. LMDB copies each touched page on write, so an idle daemon with a large graph keeps rewriting a substantial part of the store. On a copy-on-write filesystem such as btrfs every rewritten page also costs a new extent and checksum, and several idle daemons sharing one store (one per git worktree) were enough to keep IO pressure above 40% on an otherwise idle machine. `ShardedLmdb::lease` now reads the existing lease first and skips the write when it still has more than half of the lease time left; the FSDB does the same with the file's mtime. With the 2h lease time an entry is now rewritten about once an hour instead of every pass, and is still renewed at least an hour before it could expire, so garbage collection is unaffected. This addresses the write cost reported in pantsbuild#23694; the CPU cost of walking the graph on every pass, also described there, is unchanged.
… time Every 72s, pantsd's `StoreGCService` passes every digest its graph references to `Store::lease_all_recursively`, which looks up each digest's entry type in LMDB (and `stat`s the FSDB), expands every directory, and leases everything it finds. That work is the same on every pass of an idle daemon and grows with everything the daemon has loaded, so a long-lived daemon spends a steady share of a core on it (pantsbuild#23694). `lease_all_recursively` now remembers when it leased each digest it was given, and skips a digest (and everything below it) for a quarter of the lease time after that. This is safe because of the previous commit: a lease pass leaves every lease it touches with at least half of the lease time left, so a skipped digest still has at least a quarter left (30 minutes with the default 2h) when it is next renewed. Digests the graph newly references are not in the memo, so they are still leased on the next pass. That matters because storing content that already exists does not renew its lease. The pass interval is unchanged, and the walk over the graph to collect digests still runs on every pass; it is in memory.
|
One request: Please make the PR description more succinct, and move any detail (including the LLM disclosure) to a comment. The PR description becomes the commit description, and we want those to be short-ish. Once done I will merge. |
|
Moving the detail out of the description, as requested. Why idle passes were expensive
The cost grows with everything the daemon has loaded, and the writes hurt most. Every pass stores a new timestamp, so it dirties every lease page. LMDB copies each page it writes, and on a copy-on-write filesystem like btrfs each rewritten page becomes a new extent with new checksums. I run one daemon per git worktree, all sharing Why skipping is safeAfter any Storing content that already exists doesn't renew its lease, because Only pantsd's lease extension calls What I didn't do#23694 suggests renewing less often, for example every Each pass still walks the graph with MeasurementI built Pants 2.33.0a0 from source twice, once as released and once with this PR cherry-picked. For each build I started a fresh daemon in our monorepo and ran the same workload:
The "CPU spent extending leases" row subtracts an idle floor of about 2.4% of a core. I measured that floor between passes. It was the same in both builds, so it comes from pantsd's other idle work. With this PR, the first pass leases everything in 8.3s but writes nothing, since the workload had just created those leases. Every later pass finds its digests in the memo. The 12-minute window ends before the 30-minute re-expansion. I expect that pass to cost about as much as the first one, and it only rewrites leases with less than an hour left. LLM disclosureI wrote this PR with Claude (via Claude Code) and reviewed the change myself. |
|
@benjyw ok, I've shortened the description and moved the original to a comment. Hope it looks better! 🙏 |
|
Thanks for the contribution! |
`ShardedLmdb::store_bytes_batch` skips writing leases for content already stored. `CreateDigest` stores the same files on every run, so they keep their lease of their first ever write. The lease extension does not cover them, as they are rule results not a node output. GC can delete them while being actively in-use. With this PR - `Store::store_file_bytes_batch`, which only `CreateDigest` calls, now renews the lease of an already stored file under 512 KiB when it is missing or has at most half its time left, in the write transaction the batch already opens. Other writes are unchanged. - `ShardedLmdb::lease_is_stale` decides staleness in the caller's transaction. `lease_outlasts` (#23739) now calls it, and both take the half-lease threshold from `renew_before`. On an 18,000-process build that creates 5,758 files per run, this costs about 9 ms per run, and 0.08 s on a run after an idle hour. This does not cover files of 512 KiB or more, they are leased by mtime. And CreateDigest results that are kept memoized until they become old enough to become GCed (generally quite a long time since they now get leased when the rule runs if old). Disclosure: Implemented and verified with Claude with human direction
Closes #23694.
Every 72 seconds, pantsd re-expands every digest its graph references and rewrites each one's lease, even when no command has run. With several daemons sharing one store, that kept disk IO busy on an otherwise idle machine.
This makes two changes:
ShardedLmdb::leaseandShardedFSDB::leaseskip the write when the current lease has more than half the lease time left.Store::lease_all_recursivelyskips a digest, and everything below it, if it leased that digest within the last quarter of the lease time.Together they guarantee a skipped digest is renewed with about 30 minutes to spare.