Skip to content

test(persistence): immutable_reads_under_reclamation 遇到 Busy 时重试回收 - #441

Closed
jiangkoumo wants to merge 1 commit into
Ephemeral-AI-Lab:mainfrom
jiangkoumo:fix/reclaim-busy-retry
Closed

jiangkoumo wants to merge 1 commit into
Ephemeral-AI-Lab:mainfrom
jiangkoumo:fix/reclaim-busy-retry

Conversation

@jiangkoumo

Copy link
Copy Markdown
Contributor

问题
#402 的 CI(run 40058216979)在 test job 挂了 1 个用例:layerfs-persistence 的 immutable_reads_under_reclamation,第 164 行 writer.reclaim_space(1).unwrap() 拿到 Err(Busy)。其余 1087 个都过。
#402 没有改 persistence / storage / history / content,也没改 Cargo.lock,和 main 4497b0c 在这些目录零差异,所以这是 main 上本来就有的偶发失败。单独跑这个测试(代码和 main 一致),40 次挂 3 次,4 核加压时 40 次挂 1 次。

原因
Store 不设 busy handler(connection.rs 里 busy handler 是 0),回收任务碰上读线程正持有的快照时,会直接返回 Busy,什么也不做。Handles::reclaim_space 的文档也写了:被拒绝的任务不会自动重放,调用方可以之后再发一个。测试里直接 unwrap,把这个正常结果当成了错误。

改动
只改这一个测试文件:reclaim_space 返回 Busy 时 yield 一下再发下一个任务,不计入 jobs;Busy 重试次数上限 JOBS,避免死循环。其他错误照旧 unwrap。断言(位置不变、内容不变、确实回收了页、读和回收确实并发)都没动。生产代码没有改。

验证(Linux x86_64,rustc 1.85.1)
修改前:40 次挂 3 次,加压 40 次挂 1 次
修改后:80 次 0 失败,加压 40 次 0 失败;一次运行里 1147 个回收任务和 105 轮读并发
cargo test -p layerfs-persistence 全部通过;cargo clippy -p layerfs-persistence --tests -- -D warnings 通过;cargo fmt --check 通过

复现命令
cd core
cargo test -p layerfs-persistence --test immutable_reads_under_reclamation --no-run
B=$(ls target/debug/deps/immutable_reads_under_reclamation-* | grep -v ".d$" | head -1)
for i in $(seq 40); do $B -q > /dev/null 2>&1 || echo FAIL; done

…er_reclamation

The Store binds no busy handler, so reclaim_space can be refused Busy without effect while the concurrent reader holds its snapshot. The test unwrapped that outcome and failed intermittently (4 of 80 local runs, and PR Ephemeral-AI-Lab#402 CI). Treat Busy as the documented refused job and run a later one, bounded by JOBS.
@jiangkoumo
jiangkoumo marked this pull request as ready for review October 24, 2026 03:45
Copilot AI balanced review requested due to automatic review settings October 24, 2026 03:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

yifanxuaaa added a commit that referenced this pull request Oct 26, 2026
…amation

The Store binds no busy handler, so reclaim_space can be refused Busy,
without effect, when it meets the concurrent reader; the test unwrapped
that and failed intermittently (seen in #402 CI). The test now treats Busy
as the documented refused job and sends a later one, at most JOBS times;
other errors still fail and no assertion changed. Test-only; the owner
accepted this caller-side retry in a test as consistent with the
reclaim_space contract (a refused job is never replayed automatically,
a caller may issue another).

Rebased by the maintainer onto main after #465 moved the crate to
engine/, and the missing LOC line added. Checked on macOS (cargo 1.85.1):
immutable_reads_under_reclamation passes (1147 jobs during 89 read
rounds); all 36 layerfs-persistence test binaries pass (118 tests);
fmt and Clippy -D warnings clean.

Production LOC: 102165 -> 102165 (delta 0)
  First-parent comparison (main 3828268 -> this merge). core 38241,
  engine 61291, api 2633, unchanged; reference absent (retired R9c; last
  counted 65,417 at tree 498dd19). Counted with
  tools/production_loc.py; test-only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@yifanxuaaa

Copy link
Copy Markdown
Contributor

Merged into main as 41a848f, thanks! #465 moved the crate to engine/, so I rebased your commit onto main (17eea88, your authorship kept) and added the Production LOC line every commit needs (test-only: 102165 -> 102165, delta 0). This PR's branch still holds the pre-rebase commit, so GitHub shows it as closed rather than merged.

@yifanxuaaa yifanxuaaa closed this Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants