Skip to content

[PR] fix: surface real storage errors after failed BEGIN/COMMIT and auto-recover unified-store breaker (#4949) - #4955

Closed
ranxianglei wants to merge 6 commits into
masterfrom
2026-10-27_storage-breaker-recovery
Closed

ranxianglei wants to merge 6 commits into
masterfrom
2026-10-27_storage-breaker-recovery

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Closes #4949

Problem

After host storage exhaustion on v0.1.206, the status wire reported unified store auto-disabled after 3 consecutive failures (last: Error: cannot rollback - no transaction is active) — and the unified store stayed disabled even after space was freed, until process restart.

Root cause (two defects, one failure domain)

  1. Error masking. When an I/O-class failure (disk full / SQLITE_FULL) aborts BEGIN or COMMIT, SQLite auto-aborts the transaction. The node:sqlite emulation in src/storage/driver.ts then unconditionally ran ROLLBACK in its catch, which threw cannot rollback - no transaction is active — replacing the real error before it reached the breaker's lastError / status wire. Constraint-class failures were never masked (verified empirically); only I/O-class aborts are.
  2. Permanent breaker. stats.disabled was set at 3 consecutive failures and never cleared anywhere in-process — recovery required a restart.

Fix

  1. Guarded rollback in the node:sqlite emulation: the original error always wins over a secondary rollback failure. The better-sqlite3 seam annotates masked errors at our boundary (that library loses the original inside its own wrapper, so we name the likely cause instead of inventing one).
  2. Cooldown-gated writability probe (REENABLE_PROBE_COOLDOWN_MS = 5 min, constant — no new config knob): while disabled, the next save after each window elapses runs one real probe through the same transaction path production writes use (UnifiedStore.probeWritable() → single kv row in a transaction). Success re-enables the store and the triggering record ingests immediately; failure keeps the breaker armed with the real error in lastError. Both sync (node:sqlite/better-sqlite3) and async (mysql backend) sidecar paths covered.
  3. Consistency before re-enabling needs no new machinery: the load-side envelope staleness guard (persist.ts buildForLoadAll: envelope savedAt vs index row updated_at) re-imports drifted sessions, and the next save does a full ingest with raw tail — sessions missed during the outage converge automatically.

Behavior change disclosure (old → new)

  • Old: after 3 consecutive sidecar failures the unified store stayed disabled for the whole process lifetime; New: it probes writability once per 5-minute window and re-enables itself when the store heals (recovery ≤ 5 min + next save). Reason: a disk-full outage is transient by definition; permanent disablement turned a transient outage into a restart-required incident.
  • Old: I/O-class BEGIN/COMMIT failures surfaced as cannot rollback - no transaction is active; New: the original error (e.g. SQLITE_FULL: database or disk is full) surfaces. On the better-sqlite3 engine the original is lost inside the library, so the surfaced message carries an annotation naming the masking and the common cause.
  • Additive data: one fixed kv row __bili/writability-probe in index.db (single key, replaced per probe). No schema, wire-format, or persistence-version change.

Config surface

None added. The cooldown is an internal constant following the VERIFY_EVERY_INGESTS precedent (not a user-facing behavior knob).

Verification

kernel-change: n/a (no changes under kernel/)

ci-check added 2 commits October 27, 2026 18:37
…ecover the unified-store breaker (#4949)

Two defects in one failure domain, both observed in production on v0.1.206
after host storage exhaustion:

1. Error masking: when an I/O-class failure (disk full / SQLITE_FULL) aborts
   BEGIN or COMMIT, SQLite auto-aborts the transaction, so the follow-up
   ROLLBACK in the node:sqlite emulation throws 'cannot rollback - no
   transaction is active' and replaces the real error before it reaches the
   sidecar breaker's lastError / status wire. The rollback attempt is now
   guarded so the original error always wins. The better-sqlite3 seam gets
   an annotated message at our boundary because that library loses the
   original inside its own wrapper.

2. Permanent circuit breaker: stats.disabled was set on 3 consecutive
   failures but never cleared — the unified store stayed disabled for the
   whole process lifetime even after the disk was freed (only a restart
   recovered it). Both sync and async sidecar paths now run a cooldown-gated
   writability probe (REENABLE_PROBE_COOLDOWN_MS = 5min, one probe per
   window on the next save) through the same transaction path production
   writes use; success re-enables the store and the triggering record
   ingests immediately. Missed sessions converge via the existing envelope
   staleness guard at load plus the next full ingest.
- A: injected engine-level COMMIT abort over a real DatabaseSync proves the
  original error surfaces (pre-fix this failed with exactly the reported
  'cannot rollback - no transaction is active' mask); aborted rows do not
  persist.
- A2: negative control — nested transactions on a constraint failure still
  roll back cleanly and propagate the original (gate both directions, #3617).
- B: full breaker loop on a real store — 3 strikes disable with the true
  cause visible, cooldown gates probe frequency (exactly one probe per
  window), healing + one probe re-enables, triggering record lands, unified
  reads work again, exactly one RE-ENABLED log line.
- C: pins REENABLE_PROBE_COOLDOWN_MS at the documented 5-minute cadence.
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-10-27_storage-breaker-recovery (f3eb015)

Latest build — install this exact version

npm install -g billion-context@0.0.7189-pr.4955.613a1ef

This exact version is immutable and reproducible. The rolling pr tag tracks the newest test build across all PRs.

Recent builds (commit → version)

commit version build
f3eb015 billion-context@0.0.7189-pr.4955.613a1ef build
e4cd055 billion-context@0.0.6868-pr.4955.2b97c3a build
f292409 billion-context@0.0.6466-pr.4955.81a3f32 build
4508a28 billion-context@0.0.6457-pr.4955.ed6c947 build

Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr4955.tgz
npm install -g package

Install instructions are refreshed on each push.

@ranxianglei ranxianglei changed the title fix: surface real storage errors after failed BEGIN/COMMIT and auto-recover unified-store breaker (#4949) [PR] fix: surface real storage errors after failed BEGIN/COMMIT and auto-recover unified-store breaker (#4949) Oct 27, 2026
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

查重:不重复。 open+closed issue/PR 按失败签名检索(unified store auto-disabled、cannot rollback、storage breaker recovery、disk full)——除本 PR 的来源 issue #4949 外无其他条目;#4955 是 #4949 的唯一修复入口,无并列在途 PR。

这是 [PR] 接单:开始对 PR #4955 做独立审核(不复读 #4949 侧结论)——方向关(读原 issue)、回归审查(清点触碰路径的既有行为)、代码审查(逐段读 diff)、测试有效性审查。分支已拉取(4508a28),注意到 merge-guard 报过 behind,会核对与当前 master 的关系及 CI 状态。结论稍后按结构化八问回复。

@ranxianglei ranxianglei added bug Something isn't working p2 scope/storage labels Oct 27, 2026
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

Merge-guard sync done: origin/master (c499b14) merged into 2026-10-27_storage-breaker-recovery, merge commit f292409e2 pushed — PR is now 3 ahead / 0 behind, mergeable=True (mergeable_state=blocked = pending CI checks only). All 5 textual conflicts (kernel version 0.0.167→0.0.168 in package.json + both lockfiles, dsh-native #4912 decode guard, codex-catalog 304 handling) were in files this PR never touches and were resolved by taking master's side; verified the merged tree equals master plus exactly this PR's 4 files. No design decisions needed.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

CI 红判定:继承性 flake(master 自身红家族),非本 PR 引入——分支侧无代码可修,已触发失败分片重跑。

取证(run 40921903750,head e4cd055,3 个红分片逐一拉日志):

处置:按 CI 协调政策先查 #4599——master 当前红(floor 487),哨兵收敛在 origin/ci-fixes;核实 ci-fixes tip cf99d935f 已在分支基底内(merge no-op,无可推内容),且该 flake 的修复不在 ci-fixes 也不属于本 PR(舰队结论:修复合入 #5144 侧)。故按同家族先例(floor 492/493/494)对 run 40921903750 触发 rerun-failed-jobs(HTTP 201),等 attempt 2;若再现,状态留在 #4599(已评论一行),不再重复排查。

八问

  1. 是什么问题:3 个 test 分片红,每片唯一失败是 [PR] fix: recover non-retained parent fork snapshot from intact unified raw tail after cold restart (#5140) #5144 新增的 fork 用例(cold-restored parent raw tail 返回 409 而非 200)。
  2. 什么原因导致的:[PR] fix: recover non-retained parent fork snapshot from intact unified raw tail after cold restart (#5140) #5144 新测试本身阵发(flaky)——master 同一提交上既有同签名红 run 也有全绿 run,属并发时序敏感;与本 PR 改动无因果(diff 零交集)。
  3. 影响范围与严重程度:仅 CI 信号面,无用户侧功能影响;本 PR 功能(storage 熔断自愈 + 真实错误透出)不受牵连,其回归套件在本 run 全绿。
  4. 怎么修复的:无可修代码——ci-fixes 已在树内、flake 修复归属 [PR] fix: recover non-retained parent fork snapshot from intact unified raw tail after cold restart (#5140) #5144 侧;动作 = 重跑 3 个失败分片(最小代价,不推 no-op commit、不触发全量矩阵)。
  5. 是否有回归问题:不涉及——本次零代码变更;代码级回归审查已在审核轮完成(floor 7 结论,行为变更均为有意披露)。
  6. 是否破坏缓存:不涉及。
  7. 是否改了设计方向:没有。
  8. 是否引入了新的配置:没有。

为何不开 decision-request issue:auto-fix 车道要求「需设计决策则立项」,但此 flake 家族已在 #4599 完整在档(floors 485–494:证据、复现率、处置方向「修复收敛 #5144 侧」俱全),按 CI 协调政策(禁止为 CI 问题开新 issue)+ 同症状同机制归并纪律,不另立项;owner 可见性由 #4599 评论保证,非静默等待。

收工判定:等 rerun attempt 2 结果——转绿后本 PR 回到「代码审核通过(floor 7)、待 owner 合并」状态;若再现,即继承 flake 第二证,留档 #4599 待 #5144 侧收敛或 admin 重跑。

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

Synced origin/master (44067a5, incl. #5188 contention gates and #5211 revert) into this branch and pushed f3eb015 — branch is now ahead by 6 / behind by 0, mergeable=true (checks pending). Two conflicts resolved: (1) textual in src/storage/shadow.ts — both declaration blocks kept (this PR's REENABLE_PROBE_COOLDOWN_MS + #5188's isTransientContentionError); (2) a real behavioral interaction between #4949's cooldown-gated writability probe and #5188's "structural failures latch" test — with a store whose WRITE path is healthy but read-back broken, the save immediately after a fresh latch runs the probe, succeeds, and re-enables the sidecar. That is #4949's own pinned design (its test B asserts "first save after disable runs one probe"), so I updated tests/issue5140-contention-not-disabling.test.ts to assert the latch at strike 3 and pin the probe-driven re-enable as the expected outcome of the 4th save. Behavior-change flag: old = a structural 3-strike latch persisted until process restart; new = it persists only while the writability probe fails (first probe runs ungated on the next save, then once per 5-min REENABLE_PROBE_COOLDOWN_MS window); a store whose writes are broken fails its probe and stays disabled exactly as before. Local verification: typecheck clean + targeted suites green (issue4949 / issue5140 / issue5188-sidecar-recoverable-open / storage-backend-contract / issue4739 / snapshot-shadow / issue3407 / issue3919 — 37 pass, 0 fail, 3 mysql-DSN skips); full-suite verdict left to CI. State after push: open, head f3eb015, mergeable, checks running.

@ranxianglei

Copy link
Copy Markdown
Owner Author

已由人合并进 master:本修复(回滚掩码保真 + 熔断自恢复)全部并入 #5213,且熔断按 owner 终审政策整体撤销、改为永不放弃索引写入。本 PR 不再单独需要。

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

Merge-guard 收工确认:#4955 已被 #5213 取代,守卫侧不再有任何动作。

核实(独立查 API,非复读 issue 结论):

本轮及后续守卫动作:零变更 —— 不 rebase、不 push、不开 decision-request、不动分支。此前反复触发的「PR behind」唤醒现已无意义(无 open PR 可同步)。

关闭权归 owner,我不代为开关任何条目(#4955 你已自行关闭)。守卫侧到此收工。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working p2 scope/storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unified store auto-disabled after cannot rollback error following storage exhaustion

1 participant