fix(metadata): unregister() 先删存储再失效 listCache —— 删除落库前的并发 list() 不再把已删项缓存满 30s (#5259) - #5277
Merged
Merged
Conversation
…ds (#5259) `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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 7 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
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.
Fixes #5259
病灶:失效发生在「只更新了两个存储之一」的时刻
MetadataManager.unregister()原来的顺序是「删 registry →invalidateListCache(type)→await loader.delete()→ 宣告」。第 2 步与第 3 步之间是一个真实的 await 窗口(每个可写 loader 一次 DB 往返),窗口内管理器处于一个别处不存在的状态:registry 已空,loader 未空。而list()是 registry ∪ loaders 的合并,于是窗口内到达的读:listCache—— 拿满 30s 健康 TTL,因为没有任何 loader 抛错,MetadataManager.list()把「已知残缺」的降级结果照常写进 30s listCache,且listCache的注释描述的条件缓存代码里并不存在 #5184 的 2s 降级 TTL 根本不适用。删除落库后没有任何东西再失效一次(
notifyWatchers()不碰listCache),所以一个存储里已经没有的项继续被枚举最多半分钟;同一时刻get()(从不读该缓存)说它已经没了。list()是枚举面的入口(GET /api/v1/metadata/:type、Studio 左栏、同步/导出、以及所有「按已声明集合判存在性」的消费者),对permission/api这类门控类型,同一个管理器的两张脸会对「这条声明还在不在」给出相反答案。修法:靠顺序,不靠多加一次失效
register()从来没有这个毛病,原因很说明问题:它先写 registry,而合并时 registry 压过所有 loader,所以它自己的 save 窗口里合并视图已经等于写后状态。于是真正的不变量不是「早点失效」,而是:把这句话翻译到删除方向,就是存储优先:
await loader.delete()。整个窗口里 registry 和 loader 都还持有该项,所以并发list()观察到的是一个自洽的删除前状态 —— 而那就是当时的事实:删除既没落库,也没被宣告。invalidateListCache(type),两者之间不 await,所以没有任何读能插进来观察到那个半应用状态。第 1 步窗口里被缓存的、以及仍在飞的,都在这一刻——终态成立的那一刻——被一并丢弃。deleted事件叫醒、回头走list()的 watcher 拿到的是删除后状态的新鲜读。与 #5260 single-flight 的协同(这一半是承重的)
PM 要求写清「哪个机制交付了『racing 的 list() 要么看到删除后状态,要么失去缓存删除前答案的权利』」,并钉住它。答案是两个机制各管一半,缺一不可:
list()处于listCache)listCache.delete(type)inflightListReads.delete(type)—— #5260 加的撤销中间那一行是关键:光丢
listCache够不着它。在飞的读还没写条目,它会在失效之后才把删除前答案写进去。只有撤销它在inflightListReads里的登记才能拦下 —— 被撤销的读仍然为「已经在等它的调用方」返回结果(他们是在删除之前问的,#5260 明确拒绝在他们脚下重启读),但失去 memoize 的权利;而失效之后到达的调用方会发起一次全新的读,而不是加入一个删除前的读。这也解释了为什么不采用「await 前后各失效一次」:前置的那次买不到任何东西,还会重新打开第 1 步的窗口(registry 空 / loader 未空)——正是本卡的病灶本身。
loader.delete()失败路径:从warn抬到error,并说明为什么 registry 条目照删原来失败只
logger.warn('Failed to delete …')然后继续。按 AGENTS.md「Degradation log levels」的那一个问题 —— 降级之后系统从外面看是否依然正常,而它声称已持久化的东西其实没落地? —— 这里没落地的是删除,答案是「是」,而且同样安静:unregister()正常 resolve,调用方(Studio/Setup、REST DELETE、CLI、package teardown)被告知删除成功,幸存的行被下一次list()/get()直接读回来,而且永远如此,因为没有任何东西会重试。属于耐久性/一致性降级 ⇒error,并且这条error同时交付后果与修复。报一次 per 未删项,而不是 per loader 只报一次。
reportLoaderReadFailure的「只说一次」是因为list()是热路径且重复内容完全相同;这里不同 —— 每一行点名一个不同的、仍在存储里且不会被重试的项。合并它们等于把第一个受害者交给运维、把其余的名单悄悄丢掉,正是抬高日志级别要防的那件事。为什么删除失败时仍然丢掉 registry 条目(本 PR 的选择,已在代码与测试里写明理由)。 直觉上「保留条目让运行时状态与存储一致」更安全,其实不然:loader 里那一行还在,而
list()/get()是 registry ∪ loaders,所以无论保不保留,这个项都照样被端出来;保留下来唯一改变的是哪份副本赢 —— 把一份内存定义钉在一行没人再维护的存储行之上。丢掉它,下一次读就穿透到存储,而存储在删除失败之后正是事实(这个项还在),并且这个事实立刻可见(项肉眼可见地回来了),而不是等到下次重启;list()与get()也在同一时刻给同一个答案。分歧由上面那条error上报,而不是用第二份内存副本糊过去。没有改契约:
unregister()在 loader 拒绝删除时依然 resolve 而不 throw。(顺带记录一个非对称:register()的loader.save()没有 try/catch,失败会向调用方抛出;unregister()则吞掉。要不要把删除方向也改成抛出,是一个独立的公共契约问题,本卡没有要求,故明确不在此擅自决定 —— 见「留给分诊」。)让新级别被执行,而不只是被写下来
AGENTS.md 要求:发现新的耐久性缝就在同一个 PR 里把它加进
DURABILITY_CRITICAL_CALLEES。原始调用是loader.delete(...),而该 gate 按 callee 名匹配 —— 把delete放进词表会认领全仓每一个.delete()(Map、Set、缓存句柄),正是那个脚本自己头注释里警告的「假阳性多到让人把 gate 关掉」。所以这条缝被起了名字:deleteMetaItemFromLoader(与写方向的saveMetaItem对称,#4754),词表条目的爆炸半径正好是这一个调用点。(改动前是 22;新增的那一条被识别为 loud,全仓其余站点零变化。)
测试
新增
packages/metadata/src/metadata-manager-unregister-invalidate-order.test.ts,13 例。其中 6 例在origin/main的代码上失败(下面有实测输出),包括 issue 正文的探针原样搬过来那一例。测试里的 loader 有一处刻意设计:
loadMany在挂闸之前先给 store 拍快照,模拟一次「在并发删除提交之前就开始」的真实读 —— 它回答的是它看见的行,而不是它最终返回时 store 里剩下的东西。那正是「答案不许活过删除」的那一次读。反向验证(把
metadata-manager.ts换回origin/main的版本再跑同一批测试)包级全量 + 门
类型:
@objectstack/metadata在check-type-check-coverage.mjs里是 DEBT(无typecheck脚本)。用tsc --noEmit -p packages/metadata/tsconfig.json实测,origin/main基线 92 个错误,本分支 92 个 —— 本次改动新增 0 个,且我改的两个文件里一个都没有。已合并
origin/main(合到08f93bc1a),干净;packages/metadata/src/、pnpm-lock.yaml、packages/spec/在这期间都没有动过,合并后又完整跑了一遍上面的包级全量与各门。范围
严格按认领时声明的文件面:
packages/metadata/src/metadata-manager.ts—— 仅unregister()的失效/删除顺序与失败上报(外加两处同一策略块的注释:listCache字段注释与invalidateListCache()的文档,把「失效必须最后」这条不变量写在下一位作者会读到的地方);packages/metadata/src/metadata-manager-unregister-invalidate-order.test.ts;scripts/check-durability-degradation-log-level.mjs—— 只加一条词表条目(AGENTS.md 明文要求「在修复它的同一个 PR 里加」);.changeset/unregister-invalidate-after-delete.md。⛔ 未回改 #5260 的 single-flight、#5251 的 degraded/TTL、#5183 的 loader seams、#5219 的集群订阅、#5229 的 FS 失效 —— 全部是在其之上协同。
packages/spec/**零改动;content/docs/releases/**未碰;packages/core/src/**(#5257)、objectql/automation(#5038)、packages/metadata-protocol/src/protocol.ts(#5263)均未触及。越界发现(已另立卡,未在本 PR 修)
MetadataLoader不声明delete?:一个capabilities.write的 datasource loader 若没有 delete 方法,unregister()会静默跳过它并照常宣告「已删除」 #5276(finding,未认领):MetadataLoader接口声明了save?却完全没有delete,所以unregister()只能鸭子类型地探它 —— 一个声明了protocol: 'datasource:'+capabilities.write: true却没有delete方法的 loader 会被一声不吭地跳过(既不是warn,也不是本 PR 新加的error),而 registry 照删、deleted照宣告。后果与本卡处理的「delete 抛错」同类却更安静。归为 observation-class:全仓唯一的datasource:loader 是DatabaseLoader,它有delete,所以今天没有用户路径会踩到;修它要动MetadataLoader这个公共接口(第三方 loader 的实现面),故明确不在本卡内擅自决定。本 PR 只把原来的两处as any收敛成一个具名的DeletableMetadataLoader形状 —— cast 还是 cast,但至少是一个被声明的形状而不是无类型的洞;契约缺口原样留给MetadataLoader不声明delete?:一个capabilities.write的 datasource loader 若没有 delete 方法,unregister()会静默跳过它并照常宣告「已删除」 #5276。留给分诊的一个契约问题(不阻塞本卡)
register()的loader.save()失败会抛给调用方(无 try/catch),unregister()的loader.delete()失败则吞掉。本 PR 把后者从「静默」修成「响」,但没有改 throw / 不 throw 的契约,因为那会改变bulkUnregister/unregisterPackage/ REST DELETE 的既有行为,而 issue 没有要求。如果维护者认为两侧应当对称(删除失败也抛),那是一张独立的卡,我可以按吩咐去立。🤖 Generated with Claude Code
https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7
Generated by Claude Code