You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Bug] Unified snapshot flushes return before durability and reuse an expired debounce budget #6178
Package version 0.1.213, source revision 33104b1f9545caeb0344193009917ceef9969682. This is a source-level regression report for this exact revision.
Problem
Both prefix-affinity and session-order await persistUnifiedNow during shutdown, but that helper returns void while the KV read/modify/write is still queued. Graceful exit or restart can therefore proceed before the snapshot becomes durable. The unified path also leaves pendingSince set to the first batch forever, so after its max-delay expires later mutations schedule immediate writes instead of respecting the debounce window.
Proposed correction
Return the serialized KV promise to flush callers and reset the pending clock when the current batch takes ownership of its write. Later mutations receive a fresh debounce budget even if an older write is still settling. Both writers retain their existing merge, bounds and failure-demotion behavior.
Regression coverage
The prepared branch includes focused regression coverage in tests/snapshot-unified-lifecycle.test.ts, tests/affinity-persist.test.ts, tests/order-persist.test.ts, tests/affinity-unified-store.test.ts, tests/order-unified-store.test.ts.
Proposed fix: #6186. Flush callers now receive the serialized write promise, and each batch gets a fresh debounce budget. The independent persistence suite passes 28/28, including shutdown durability and later-batch scheduling regressions.
查重 (dedup): Not a duplicate. Searched open AND closed issues+PRs for this mechanism (unified-snapshot flush durability / debounce-budget reuse): only this thread and its fix PR cover it. Nearest lineage — #5753 (the unified-storage migration that introduced these kv writers), #5757 (its failure-demotion review fix), #6066 (debounce/max-delay knobs) — none reports this defect.
Bug verified at the pinned revision (33104b1f, v0.1.213 — file-identical to current master for both affected files). Both reported defects exist exactly as described:
Flush returns before durability — persistUnifiedNow() in src/order-persist.ts:164 and src/affinity-persist.ts:166 returns void while queueing the KV read-modify-write onto kvChain. The shutdown path (src/server.ts:1757-1758) and both restart paths (src/restart.ts:488-489, 498-499) await it, so the await is a no-op and process.exit(0) can run while the snapshot write is still queued/in-flight. Affects default installs — unified sqlite storage is the default carrier, and bili's routine self-update restarts go through exactly this flush sequence.
Expired debounce budget reused — pendingSince is cleared only on the legacy sync path (writeSnapshotLegacy's finally) and at flush; nothing resets it after a unified batch fires. Once the first batch's max-delay expires, every later mutation computes a non-positive budget → zero-delay timer → one RMW per mutation instead of the documented ~500 ms quiesce / 5 s cap cadence (/bili/url/重启agent、重启bili、开新会话分支啥的都有概率丢失压缩 #1834).
Labels applied:bug, scope/storage, p1 — on both this issue and PR #6186. Reporter suggested p2; I labeled p1 because silently losing durable state on a routine graceful exit/restart is data-loss class, which owner policy (2026-10-19) keeps at p1. (An earlier daemon pass reported these labels but could not apply them; they are now applied.)
Fix: your PR #6186 targets this thread, so per the one-fix-PR-per-thread rule I reviewed it instead of opening a parallel one. Independent verification, redone locally just now at head 851e3bb766 (not taken from the description):
Targeted suites — new tests/snapshot-unified-lifecycle.test.ts plus the four adjacent persistence suites: 28/28 pass.
Negative control — reverting ONLY the two production files to their parent version: all 4 new regression cases fail (each on its named assertion); restoring the fix: 4/4 pass.
Base drift: master has moved since the PR base e3220adc; no intervening commit touches the three touched files, so the diff still applies cleanly (GitHub's "behind" flag is base-sha staleness only).
The full structured review — behavior-change disclosures (old→new), test-quality assessment, §7.4 auto-merge gate verdict, and a merge-ordering coordination note for the open immutable-journal PR #5480 (same two source files, whose helper still returns void) — is on the PR thread: #6186.
Verdict: ready to merge. Merging itself stays human.
Closing status — full triage + structured review posted above in this thread; outcome:
查重: not a duplicate (details above).
Bug confirmed at pinned revision 33104b1f (v0.1.213): both reported defects are real — shutdown/restart awaits on persistUnifiedNow() are no-ops against the queued KV writes, and the unified-path debounce clock never resets after a batch fires.
Fix: your PR [PR] fix: await unified snapshot writes and reset each debounce batch #6186 targets this thread — reviewed it, did not open a parallel PR. Independently verified at head 851e3bb766: 28/28 targeted persistence suites pass, negative control 4/4 fail-without-fix → 4/4 pass-with-fix, tsc clean, remote CI green (30/30), no master drift on touched files.
Labels applied: bug, scope/storage, p1 (data-loss class per owner policy 2026-10-19) on issue and PR.
Activity
Afloat16 commentedon Nov 11, 2026
Proposed fix: #6186. Flush callers now receive the serialized write promise, and each batch gets a fresh debounce budget. The independent persistence suite passes 28/28, including shutdown durability and later-batch scheduling regressions.
ranxianglei commentedon Nov 11, 2026
(ai: vllm-qwen/qwen3.8-27b)
查重 (dedup): Not a duplicate. Searched open AND closed issues+PRs for this mechanism (unified-snapshot flush durability / debounce-budget reuse): only this thread and its fix PR cover it. Nearest lineage — #5753 (the unified-storage migration that introduced these kv writers), #5757 (its failure-demotion review fix), #6066 (debounce/max-delay knobs) — none reports this defect.
Bug verified at the pinned revision (
33104b1f, v0.1.213 — file-identical to current master for both affected files). Both reported defects exist exactly as described:persistUnifiedNow()insrc/order-persist.ts:164andsrc/affinity-persist.ts:166returnsvoidwhile queueing the KV read-modify-write ontokvChain. The shutdown path (src/server.ts:1757-1758) and both restart paths (src/restart.ts:488-489, 498-499)awaitit, so the await is a no-op andprocess.exit(0)can run while the snapshot write is still queued/in-flight. Affects default installs — unified sqlite storage is the default carrier, and bili's routine self-update restarts go through exactly this flush sequence.pendingSinceis cleared only on the legacy sync path (writeSnapshotLegacy'sfinally) and at flush; nothing resets it after a unified batch fires. Once the first batch's max-delay expires, every later mutation computes a non-positive budget → zero-delay timer → one RMW per mutation instead of the documented ~500 ms quiesce / 5 s cap cadence (/bili/url/重启agent、重启bili、开新会话分支啥的都有概率丢失压缩 #1834).Labels applied:
bug,scope/storage,p1— on both this issue and PR #6186. Reporter suggested p2; I labeled p1 because silently losing durable state on a routine graceful exit/restart is data-loss class, which owner policy (2026-10-19) keeps at p1. (An earlier daemon pass reported these labels but could not apply them; they are now applied.)Fix: your PR #6186 targets this thread, so per the one-fix-PR-per-thread rule I reviewed it instead of opening a parallel one. Independent verification, redone locally just now at head
851e3bb766(not taken from the description):tests/snapshot-unified-lifecycle.test.tsplus the four adjacent persistence suites: 28/28 pass.tsc --noEmitclean on the PR head tree.e3220adc; no intervening commit touches the three touched files, so the diff still applies cleanly (GitHub's "behind" flag is base-sha staleness only).The full structured review — behavior-change disclosures (old→new), test-quality assessment, §7.4 auto-merge gate verdict, and a merge-ordering coordination note for the open immutable-journal PR #5480 (same two source files, whose helper still returns void) — is on the PR thread: #6186.
Verdict: ready to merge. Merging itself stays human.
结构化八问:
await对这条队列是空操作——优雅退出可能丢掉最近的状态;同时去抖时钟在批次落盘后不复位,持续流量下之后每次变更都触发立即写库。persistUnifiedNow()返回 void,调用方的await没有可等的 promise;(b)pendingSince只在 legacy 同步路径清除,统一路径永远钉死在第一批的时间戳上,预算算出负数 → 定时器延迟被钳到 0。kvChainpromise,使 flush 调用方真正等到全部排队写入落定;批次接管写入时清空pendingSince,后续变更获得全新的去抖预算。合并/限幅/失败降级逻辑一律不动。withExitDeadline约束)、失败降级(不变,catch 仍正常结算链,flush 永不抛错)、合并/限幅(未触碰)、legacy 文件路径(未触碰)、hydrate 流程(未触碰)。无 fail-fast↔吞错反转、无阈值/时序漂移(除已披露项)、无输出格式变化。未发现回归。flushResignPending已有的正确模式对齐)。ranxianglei commentedon Nov 11, 2026
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b)
Closing status — full triage + structured review posted above in this thread; outcome:
33104b1f(v0.1.213): both reported defects are real — shutdown/restartawaits onpersistUnifiedNow()are no-ops against the queued KV writes, and the unified-path debounce clock never resets after a batch fires.851e3bb766: 28/28 targeted persistence suites pass, negative control 4/4 fail-without-fix → 4/4 pass-with-fix,tscclean, remote CI green (30/30), no master drift on touched files.bug,scope/storage,p1(data-loss class per owner policy 2026-10-19) on issue and PR.