Repository navigation
[PR] fix(storage): don't leave archive rows on cleanup failure; use the latest matching row on recovery | fix(storage): 清扫失败不留归档行;恢复时取最新的匹配行 - #6318
Conversation
…matching entry archiveSession appended the session's export line to archive.jsonl and only then ran the sweep transaction. When the sweep failed (write lock held by another process past busy_timeout, a constraint error), the line stayed in the log while the session stayed active and kept growing. The next successful archive appended a second line; restoreArchived took the first matching line, i.e. the stale one, and then removed every line of that session, so the newer export was lost. Each failed attempt (gc retries included) also added another full copy of the session to the log. - archiveSession (sqlite + mysql): serialize the line before the sweep, append it as the last step inside the sweep transaction. A sweep that cannot start or fails midway writes nothing; the line is still on disk before COMMIT, so a committed sweep always has its export. - restoreArchived (sqlite + mysql): scan the log from the end so the newest line of a session wins, and warn when more than one line of that session is dropped. Logs written by earlier versions can already hold such pairs. - tests/storage-archive-failed-sweep.test.ts: 4 cases (failed sweep leaves no line and the later archive restores in full; newest line wins; both for sqlite and for the mysql backend on a stub driver).
There was a problem hiding this comment.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🔵 Needs a closer look
The review found an issue that should be addressed before approval.
No open issues 🎉
What changed in this PR
Fixes an archive/restore correctness bug in the storage layer where archiveSession could leave stale archive.jsonl rows on sweep failure and restoreArchived could then restore from an outdated export and delete the newer one.
Changes:
- Move session archive-log append into the sweep transaction (SQLite + MySQL) so failed sweeps don’t leave stray archive rows.
- Make
restoreArchivedprefer the newest matching JSONL entry and warn when multiple entries exist for the same session. - Add regression tests covering failed sweeps and multi-entry recovery for both backends (MySQL via stubbed driver).
| File | Description |
|---|---|
| tests/storage-archive-failed-sweep.test.ts | Adds regression coverage for failed sweep behavior and “newest entry wins” restore semantics across SQLite and mocked-MySQL. |
| src/storage/store.ts | Appends archive JSONL line as the last step inside the SQLite sweep transaction; restores by scanning JSONL from end and warns on duplicate session entries. |
| src/storage/mysql.ts | Mirrors the SQLite changes for MySQL: append within transaction and restore using the newest matching JSONL entry (plus duplicate-entry warning). |
Review effort: Balanced
|
该PR已按根因归类纳入 [advisory-update] 根因汇总 #6342,由汇总单统一跟踪处理;本单不再单独跟进,故关闭。如认为归类有误或问题仍未解决,请直接回复,会重新打开。 |
|
状态更正(自动化脚本误操作): 本单此前被「纳入根因聚合」流程误关闭, 现重新打开。机器人工作已停, 本单由根因聚合线程统一跟踪, 根因修复落地后请手动关闭或在本单回复「close」。 |
令人唏嘘 |
|
重新打开:此前被归类聚合脚本误关。按 owner 口径,纳入根因聚合的 issue 只停机器人、不关闭,由根因聚合线程统一跟踪,根因修复落地后统一关闭。 |
|
(更正)此前按"清理"口径误关闭;按 owner 口径纠正:被根因聚合纳入的 issue 只停机器人、不关闭。本 issue 重新打开,机器人工作已停。 |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) 查重: 不重复。 已搜 open+closed 的 issue/PR(archive.jsonl / archiveSession / restoreArchived / 归档 / 恢复):没有对「先追加后清扫 + 恢复取第一条匹配」这一缺陷的既有报告或修复;#6317 是本 PR 的源 issue(同作者、同缺陷);#6200/#6201(恢复后重写日志的原子性)、#6230/#6245(JSONL 非对象条目)、#6196/#6198(恢复时的内容归属)同函数族但症状与机制均不同,PR 正文已逐一交叉引用。 标签: 按 2026-11-12 硬规则(不改动本任务所指源 issue/PR 的标题与标签),未对本单和 PR 打标签。我的 triage 判定供参考: bug / scope/storage / p1(恢复时静默数据丢失,数据丢失类)。 方向审核(原 issue): #6317 的复现在代码里成立—— 回归审查(触碰路径的既有行为逐项清点):
代码审核: 事务语义已核实——better-sqlite3 的 测试有效性: 4 个用例都是真实注入(sqlite 用触发器打进真代码路径 / mysql 用桩驱动记录调用序列),不是 mock 被测函数。我本地实跑: PR head 上 4/4 通过;未改动的基线 c26c31a 上 4/4 失败——测试确实抓得住原 bug;mysql 用例还钉住了「行在 COMMIT 前已落盘」。既有 archive/GC 测试(storage-archive-gc、issue4665、issue4796)15 过 + 1 skip(真 MySQL DSN 门,CI mysql-contract 覆盖)。 状态(已 live 核实): PR head(047da26f)CI 全绿——30 checks, 0 failed, 0 pending(test lanes ubuntu/windows × node 22/24、static-gates、mysql-contract、kernel-guard、build-artifact、gate 全 success;e2e lanes 因 diff 不碰其触发文件而 skipped)。origin/master 已比 PR 基线前进 5 个提交(#6326 durability-before-render),与本 PR 无文件重叠、无冲突(mergeable=true)。本单曾被聚合脚本误关、已按 owner 更正重开,现由根因聚合 #6342 统一跟踪;按政策我不关闭本单,也未动其标题/标签。 其他观察(既有行为,本 PR 未改变,不拦): gc() 的 age-sweep 循环无 try/catch,单个会话 archiveSession 抛错会中断整个 gc(剩余会话 + zero-ref 收集 + vacuum 全部跳过)——改动前后行为相同。 八问
结论: 可合(head 上 CI 全绿;4 个新用例基线失败/head 通过已本地核实;既有 archive/GC 测试全绿)。按规则合并由人工执行: #6318 |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) 状态更新(完整 review 见本单上一条评论,不重复):
|
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) 收到,对归类无异议:本 PR 的修复经审核正确、可合(完整 review 与八问见本单 08:16 的评论,不重复),纳入根因汇总 #6342 统一跟踪合理。 当前状态(已 live 核实):
|
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) (重复派单,工作已完成,仅确认现状,无新增动作)
|
Fixes #6317
问题
archiveSession先把会话的导出行追加到archive.jsonl,再跑清扫事务。清扫失败时(写锁被另一个进程占着超过busy_timeout、约束错误、进程在两步之间死掉),这一行已经进了日志,而会话仍是active、还在继续用。这个会话下一次归档成功时会再追加一行。
restoreArchived从头往下扫日志,取第一条sessionId匹配的行(也就是过期的那条)来恢复,然后把这个会话的所有行从日志里去掉,较新的导出也跟着没了:恢复出来的会话悄悄少了失败那次和真正归档之间新加的全部内容。另一个影响:每次失败的尝试(包括 gc 的重试)都会往
archive.jsonl里再加一份完整的会话。两个后端(
src/storage/store.ts、src/storage/mysql.ts)都是同样的顺序、同样的取第一条匹配。改动
archiveSession(sqlite + mysql):这一行照旧在清扫之前序列化,但改为在清扫事务里面作为最后一步追加。清扫开不了头或中途失败就什么都不写。这一行仍然在COMMIT之前写进日志,所以提交了的清扫一定有它的导出;追加失败则清扫回滚。restoreArchived(sqlite + mysql):从日志末尾往前扫,同一会话以最新的一行为准。被恢复的会话去掉的行多于一行时,打一条warn日志说明。这样既照顾到旧版本写下的日志,也照顾到剩下的那个窄窗口:追加成功了,但COMMIT本身失败或进程在它之前死掉。行格式、导出路径和返回值都不变。
追加留在
COMMIT之前,没有挪到之后:在提交了的清扫之后再追加,会打开反方向的窗口(库里的行已清掉、盘上却没有导出),丢的是整个会话,而不只是留下一条过期的重复行。本 PR 没有改动
kernel/**。测试
tests/storage-archive-failed-sweep.test.ts,4 个用例:COMMIT之前已经落盘。在未改动的 master(
c26c31ace)上 4 个全部失败;加上改动后 4 个全部通过。验证
npm run typecheck、check:unused-exports、check:test-import-graph、tsup构建及两项 dist 检查、git diff --check:全部干净。BEGIN IMMEDIATE,archiveSession报database is locked失败):master 上之后的恢复只拿回四条消息中的两条,改动后四条全部拿回。tests/*.test.ts,Windows / Node 22,网络被沙箱隔离):没有因这次改动而失败的。把受影响的文件串行重跑后仍然失败的那些都和本机环境有关(杀毒软件拦截回环 TLS、本机不在 UTC、沙箱屏蔽了 DNS / 外网),在 master 上也是同样失败。以推送后 head 上的 CI 为准(§7.2)。mysql2模块。相关,但不重叠