perf(storage): avoid scheduled-task catalog rewrites - #4955
Conversation
Use keyed task and fire-claim reads and row-level writes within the existing serialized SQLite transaction. Keep task settlement and claim removal atomic, and preserve execution identity and native admission rules. Keep due discovery as an explicit catalog scan, writing only newly expired tasks and a new claim when present. Unchanged polls perform no DML. Validate task row identities before expiry updates. Add regression coverage for indexed point operations as unrelated data grows, no-op writes, expiry, identity corruption, rollback and owner reopen. Fixes apache#4954 Refs: apache#4876 Generated-by: Codex
hqhq1025
left a comment
There was a problem hiding this comment.
复核 10e8f42a87a6e210c51470d45d36061ffb1bcc31,未发现有充分证据支持的 P0–P3 问题。局部数据库工作量下降有可复核证据;是否接受性能重构仍交人类维护者决定。
实际改动把整张 task/claim 表的读取和重写改为按现有索引访问及行级更新。scheduled-task-store.ts:243 的点读只读目标任务,:617 保留串行队列并把整个操作放进 SQLite write transaction;:500 在同一事务更新当前任务并删除领取记录。:382 的到期扫描仍遍历任务目录,只写新过期项及至多一条新领取;:628 先核对 JSON 与行键身份,防止更新错误任务。现有主键及 claim.task_id 唯一索引已覆盖这些 SQL,数据库 schema 未变,无 migration。
干净安装、build:test、Storage 类型检查、变更文件 lint/format、20 项 Storage/Host 相关回归通过。真实 owner lease 和 SQLite 路径覆盖写入成本、事务中途故障回滚、重新获取 owner、待交付通知及执行身份。额外并发探针验证 32 次同时扫描只生成一个领取,两次同时结算只成功一次,16 次同时 snooze 累积不丢更新,拒绝后队列仍可用。
独立将同一成本夹具运行于 base 实现,256 个无关任务及领取时点读实际返回 513 条 JSON;当前实现的上限断言为 1 条。原实现的空扫描确有表级 DML,新实现重复空扫描为零 DML。因此收益不只来自 SQL 文本推断。
此结论不包含生产延迟基准或整个 Host 调度周期的有界性:scheduled-task-coordinator.ts:530、:813 仍读取完整目录,点操作也有意不再解码无关记录。未运行所有工作区或 Windows 原生执行。当前 head 的 hosted test 成功,基于当前 main b06eb02e6,合并树无冲突。
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the focused storage change. Reviewed 10e8f42: no P0–P3 findings. Keyed operations retain the existing serialized SQLite authority; task settlement and claim deletion remain atomic, failed writes leave the queue usable, and unchanged due scans perform no DML. Current-head CI passes and the integration tree merges cleanly. Validation includes source review and an independent reviewer; the existing exact-head review records owner/SQLite regression and concurrency probes. My attempted local rerun could not start because the reused dependency installation is no longer available; it is not counted as a test pass. AI-assisted review.
中文
感谢这次聚焦的存储优化。复核当前 head,无 P0–P3。点操作保留现有串行 SQLite 权威;任务结算与 claim 删除保持原子性,写入失败后队列可继续,未变化的到期扫描不写库。当前 CI 通过,合成合并无冲突。本次有源码及独立 reviewer 核对,已有同 head 评审记录真实 owner/SQLite 回归和并发探针。我尝试的本地重跑因复用依赖已不可用而未能启动,不计作测试通过。AI 辅助评审。
Summary
A single scheduled-task mutation previously loaded and rewrote both the task and fire-claim tables; even an unchanged due scan did so. Use indexed task/claim reads and row-level writes inside the existing serialized SQLite transaction. Settlement updates the current task and removes its claim atomically. Due discovery writes only newly expired tasks and at most one new claim, and validates task identities before applying expiry updates.
With 256 unrelated tasks and 256 pending claims created through the store API, a point read now returns 1 JSON record instead of 513, snooze changes 1 row instead of 1,026, and an unchanged poll changes 0 rows instead of 1,026.
Due discovery remains a catalog traversal, and the Host's schedule/residency refresh still uses full lists. No schema or protocol change. Point operations no longer decode unrelated records; full-list reads still validate the records they load.
Fixes #4954
Context: Discussion #4876.
Verification
Local macOS verification with Node 24.18.0 and npm 11.19.0:
npm run build:test; full productionnpm run buildalso passed earlier with Node 22.23.1.git diff --checkpassed.Fixtures use the real owner lease and store API. SQL observation covers returned payload rows, actual query plans and DML changes; fault injection covers mid-settlement rollback and damaged identities. Reopening releases and reacquires the owner. These checks do not claim a bounded whole-Host polling path or measure production latency.
Not run: every workspace's tests, Windows execution, or the Electron/packaged app. Both directly affected workspace suites were run in full on macOS.
AI use
Tool(s) and scope: Codex investigated the defect, authored the implementation and regression tests, ran local verification, and prepared the contribution drafts. Submitted with contributor approval; upstream review is pending.
Checklist
Does this PR entail a change in behavior?