fix(agfs-fuse): bound metadata caches and stop cleanup goroutine cleanly - #24
Merged
Conversation
`agfs-fuse/pkg/cache.Cache` was TTL-only with no upper bound and no
shutdown signal for its background cleanup goroutine:
- Long-lived mounts could grow memory without bound between TTL ticks
whenever the access pattern produced distinct keys faster than they
expired (e.g. crawling a large tree).
- `Cache.cleanup` ran on `for range ticker.C` with no exit path, so the
goroutine outlived the cache for the rest of the process lifetime.
This change adds optional LRU bounding and a deterministic shutdown
signal, and opts the FUSE filesystem into both with conservative
defaults.
Production changes:
- `pkg/cache/cache.go`:
* `Cache` is now backed by `container/list` + a map of
`key -> *list.Element` so eviction is O(1) and `Get` promotes the
entry to the front of the LRU list on hit. The contained `entry`
embeds its key so eviction can locate its map slot without scanning.
* Functional-options pattern: `NewCache(ttl, opts ...Option)`. The
only option today is `WithMaxEntries(n)`; <= 0 keeps the legacy
unbounded behaviour for direct callers that haven't opted in.
* `Set` evicts the LRU entry when over capacity. Updating an existing
key is treated as an in-place update — it does not count as a new
insertion for capacity purposes and does not evict a still-present
sibling.
* `Get` on an expired entry returns `(nil, false)` and prunes it
immediately rather than waiting for the next sweep, so callers
between ticker fires still see correct expiration semantics.
* `Stop()` (sync.Once-guarded) closes a `stop` channel; the cleanup
goroutine ranges on a `select` that exits when `stop` is closed.
As the last thing on its way out, `cleanup` closes a `done`
channel exposed via `Cache.Done()` so callers can wait
deterministically — useful for tests and strict shutdown ordering.
* Added a public `Len()` helper that returns the current entry count
(mainly for tests, but also useful for telemetry).
* `MetadataCache` and `DirectoryCache` now thread `Option`s through to
the underlying `Cache` and expose their own `Stop()` for the
containing filesystem to call on shutdown.
- `pkg/fusefs/fs.go`:
* `AGFSFS.Close()` now calls `metaCache.Stop()` / `dirCache.Stop()`
in addition to clearing them, so the FUSE filesystem shuts down
without leaking the cache cleanup goroutines.
* `Config` exposes `MetaCacheMaxEntries` and `DirCacheMaxEntries`,
both configurable. Defaults are conservative: 50_000 metadata
entries and 5_000 directory listings. Rationale:
- 50_000 metadata × ~200 B/entry (rough `FileInfo` size, name +
a few ints) ≈ 10 MB worst-case for the metadata path;
- directory listings carry slices of `FileInfo`, so the cap is an
order of magnitude tighter to keep the total bounded;
- both leave comfortable headroom for interactive workloads while
preventing the unbounded-growth failure mode.
Override via `Config` if a deployment needs more or less.
Tests (additive — all existing cache tests still pass unchanged):
- `TestCacheLRUEviction` — Set a→1, b→2, Get(a), Set c=3. b is now LRU
(since a was just touched) and gets evicted. Asserts a/c survive,
Len() == 2.
- `TestCacheLRUDoesNotEvictWithoutLimit` — backwards-compat pin: the
legacy `NewCache(ttl)` path still admits unlimited entries.
- `TestCacheUpdateInPlaceDoesNotEvict` — Set a/b, Set a again. a is
still 99 and b is still present (eviction would have been wrong).
- `TestCacheStopExitsCleanupGoroutine` — calls Stop, asserts
`<-Done()` returns within 2s. Deterministic shutdown signal, no
`runtime.NumGoroutine` polling, no flakes.
- `TestCacheStopIdempotent` — 8 goroutines call Stop concurrently;
asserts no panic and Done() still closes. Pins the sync.Once guard.
- `TestCacheExpiredEntryReturnsNotFound` — TTL 10ms; sleep 50ms; Get
on the expired key returns (nil, false) and Len() == 0 even though
the sweeper may not have run yet.
- `TestMetadataCacheStop` / `TestDirectoryCacheStop` — confirm the
wrapper types forward Stop() to the underlying Cache.
Verification:
go test ./pkg/cache -v -timeout 30s # 14 PASS
go test ./... -timeout 60s # all PASS
go test -race ./pkg/cache -timeout 30s # PASS
Closes the FUSE half of the diagnostics P0 ("bound FUSE cache and stop
cleanup goroutine cleanly").
Merged
This was referenced May 11, 2026
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.
Summary
Stop()/Done()shutdown for cache cleanup goroutinesAGFSFSVerification
go test ./pkg/cache -v -timeout 30s-> 14 PASSgo test ./... -timeout 60s-> passgo test -race ./pkg/cache -timeout 30s-> passgit diff --check origin/master..HEAD-> cleanNotes
Cache caps are configurable through
fusefs.Config; CLI flags for runtime tuning are a non-blocking follow-up if operators need them.