Skip to content

Commit e92e2c3

Browse files
os-zhuangclaude
andauthored
fix(metadata): invalidate the list cache AFTER the storage delete lands (#5259) (#5277)
`MetadataManager.unregister()` dropped the registry entry and called `invalidateListCache(type)` BEFORE awaiting `loader.delete()`. Those two steps are separated by a real await window (one DB round-trip per writable loader), and inside it the manager held a state that exists nowhere else: registry already empty, loader not yet empty. `list()` merges the two, so a read arriving in that window missed the just-cleared cache, assembled the still-stored row into its answer, and memoized it as a COMPLETE read — the full 30s healthy TTL, because no loader threw and #5184's 2s degraded TTL therefore never applied. Nothing invalidated again once the delete landed (`notifyWatchers()` does not touch `listCache`), so an item gone from storage kept being enumerated for up to half a minute while `get()` said it was gone. Fixed by ordering, not by a second invalidation. `register()` never had this defect because it writes the registry first and the registry outranks every loader in the merge, so its save window already shows the post-write state. The invariant is therefore not "invalidate early" but invalidate LAST, once every store already holds the state being announced. `unregister()` now deletes from storage first, then drops the registry entry and invalidates with nothing awaited between them, then publishes and announces (#5219's invalidate-before-notify bar, unchanged). Composes with #5253's single-flight rather than duplicating it: a read still in flight when the delete lands cannot be reached by dropping `listCache` — it has not written its entry yet and would write the pre-delete answer afterwards. `invalidateListCache()` also retracts that read's `inflightListReads` registration, so it resolves for the callers already waiting on it but loses the right to memoize, while a caller arriving later starts a fresh read. A failing `loader.delete()` used to `logger.warn` and continue. Per AGENTS.md "Degradation log levels" that is durability degradation, not functional: `unregister()` resolves normally, the caller is told the delete succeeded, and the surviving row is read straight back out of storage — permanently, since nothing retries it. It now logs at `error`, once per un-deleted item, naming the consequence and the fix, and the seam is named `deleteMetaItemFromLoader` so `check:durability-log-level` covers it (22 -> 23 seams, all loud). The registry entry is still dropped in that case, deliberately: the loader still holds the row so the item is served either way, and keeping the entry would only pin an in-memory copy on top of a stored row nobody maintains. Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0f2fdcd commit e92e2c3

4 files changed

Lines changed: 762 additions & 13 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@objectstack/metadata": patch
3+
---
4+
5+
fix(metadata): `unregister()` invalidates the list cache AFTER the storage delete lands (#5259)
6+
7+
`MetadataManager.unregister()` dropped the registry entry and called
8+
`invalidateListCache(type)` **before** awaiting `loader.delete()`. Those two steps
9+
are separated by a real await window — one DB round-trip per writable loader — and
10+
inside it the manager held a state that exists nowhere else: **registry already
11+
empty, loader not yet empty**. `list()` merges the two, so a read arriving in that
12+
window missed the just-cleared cache, assembled the still-stored row into its
13+
answer, and memoized it as a *complete* read — the full 30s healthy TTL, because no
14+
loader threw and #5184's 2s degraded TTL therefore never applied.
15+
16+
Nothing invalidated again once the delete landed (`notifyWatchers()` does not touch
17+
`listCache`), so an item that was gone from storage kept being enumerated for up to
18+
half a minute. `list()` is the enumeration seam behind `GET /api/v1/metadata/:type`,
19+
the Studio left rail, sync/export and every consumer that decides existence from a
20+
declared set — and `get()`, which never reads that cache, said the item was gone the
21+
whole time. For a gating type (`permission`, `api`) the two faces of one manager
22+
answered opposite questions about whether a declaration exists.
23+
24+
**Fixed by ordering, not by an extra invalidation.** `register()` never had this
25+
defect because it writes the registry *first* and the registry outranks every loader
26+
in the merge, so its own save window already shows the post-write state. The
27+
invariant is therefore not "invalidate early" but *invalidate last, once every store
28+
already holds the announced state*. `unregister()` now deletes from storage first,
29+
then drops the registry entry and invalidates with **nothing awaited between them**,
30+
then publishes and announces — #5219's invalidate-before-notify discipline unchanged.
31+
A `list()` racing the delete now either sees a coherent pre-delete state (the delete
32+
has not landed and has not been announced — that answer is the truth) or the
33+
post-delete state; it can no longer cache the pre-delete answer past the delete.
34+
35+
This composes with #5253's single-flight rather than duplicating it: a read still
36+
*in flight* when the delete lands cannot be reached by dropping `listCache` — it has
37+
not written its entry yet and would write the pre-delete answer afterwards.
38+
`invalidateListCache()` also retracts that read's `inflightListReads` registration,
39+
so it resolves for the callers already waiting on it but loses the right to memoize,
40+
while a caller arriving later starts a fresh read.
41+
42+
**A storage delete that fails is now loud.** It used to `logger.warn('Failed to
43+
delete …')` and continue. Per AGENTS.md "Degradation log levels" this is
44+
durability/consistency degradation, not functional: `unregister()` resolves
45+
normally, the caller is told the delete succeeded, and the surviving row is read
46+
straight back out of storage by the very next `list()`/`get()` — permanently, since
47+
nothing retries it. It now logs at `error`, once per un-deleted item, naming the
48+
consequence and the fix. The registry entry is still dropped in that case,
49+
deliberately: the loader still holds the row so the item is served either way, and
50+
keeping the entry would only pin an in-memory copy on top of a stored row nobody
51+
maintains — dropping it makes the next read fall through to storage, which is the
52+
actual truth after a failed delete, and makes it visible immediately instead of at
53+
the next restart.
54+
55+
No API change. `unregister()` still resolves rather than throwing when a loader
56+
refuses the delete.

0 commit comments

Comments
 (0)