fix(boot-profile): close foyer clean cache before TempDir drop (stops tmpfs fd leak)#85
Merged
Merged
Conversation
The boot-set profiler (`capture_once`) opens a per-run `FoyerBlockCache` in a `tempfile::TempDir` and never closes it — it only Arc-drops at function return. foyer's SSD-tier region files stay open until the storage engine is closed, so the `TempDir`'s `remove_dir_all` runs while the fds are still held, orphaning them as deleted-but-held files that are never reclaimed until the daemon process exits. A long-lived daemon that profiles many images (instd drives a profile run per rootfs compose/create) leaks these unbounded into `$TMPDIR`. On hosts where `/tmp` is a tmpfs this fills RAM and ENOSPCs every other process on the node. Observed in the field: 5472 deleted 16 MiB regions (~85 GiB logical) pinning a 14 GB tmpfs at 100%; a daemon restart reclaimed it all. Add `BlockCache::close()` (default no-op; in-memory impls hold no fds), implement it on `FoyerBlockCache` via foyer's `HybridCache::close()`, and call it in `capture_once` before the `TempDir` is dropped, on every exit path (the work is wrapped in a `result` async block so early `None`s still close). The TempDir cleanup then actually frees the region files. Note: this fixes the fd leak. Placing the profiler scratch on the configured `[cache].dir` instead of `std::env::temp_dir()` is a separate follow-up; operators can currently point `$TMPDIR` at a real-disk path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| } | ||
|
|
||
| async fn close(&self) { | ||
| if let Err(e) = self.inner.close().await { |
Contributor
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
FoyerBlockCache::close logs and suppresses self.inner.close() failures, so capture_once cannot detect that cleanup did not complete and may still leave deleted-but-held region files pinned in $TMPDIR; this weakens the new leak fix under error conditions. Return a Result from BlockCache::close and propagate/handle it at the call site so failed shutdown is explicit and retryable instead of silently continuing.
// glidefs/src/block/cache.rs
async fn close(&self) {
if let Err(e) = self.inner.close().await {
tracing::warn!(error = %e, "foyer clean cache close failed");
}
}
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
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.
Problem
The boot-set profiler (
oci/boot_capture_served.rs::capture_once) opens a per-runFoyerBlockCachein atempfile::TempDirand never closes it — only Arc-drop at function return. foyer's SSD-tier region files stay open until the storage engine is closed, so theTempDir'sremove_dir_allruns while the fds are still held, orphaning them as deleted-but-held files that aren't reclaimed until the process exits.instd drives a profile run per rootfs compose/create, so a long-lived daemon leaks these unbounded into
$TMPDIR. On hosts where/tmpis a tmpfs this fills RAM and ENOSPCs every other process on the node.Observed in the field (glidefs 1.0.1):
A daemon restart reclaimed all of it (confirming held fds, not on-disk files).
Fix
BlockCache::close()(default no-op; in-memory impls hold no fds).FoyerBlockCachevia foyer'sHybridCache::close().capture_oncebefore theTempDirdrops, on every exit path (the body is wrapped in aresultasync block so the earlyNonereturns still close). The TempDir cleanup then actually frees the regions.Notes / follow-up
This fixes the fd leak. Separately, the profiler scratch uses
std::env::temp_dir()(ignores[cache].dir); placing it on the configured cache filesystem is a follow-up. Operators can currently work around the tmpfs placement withTMPDIR=<real-disk>in the unit.Test
cargo check -p glidefsclean. Theresultasync block preserves the original control flow (.ok()?/return None) and output type (Option<Vec<u64>>).