Repository navigation
fix(runtime-core): let shutdown close wait for draining leases - #4141
Conversation
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if crate::native_runtime::sleep_until( | ||
| std::time::Instant::now() + SHUTDOWN_HOLD_DRAIN_POLL, | ||
| ) | ||
| .await | ||
| .is_err() | ||
| { | ||
| return self | ||
| .close_idle_batch(true) | ||
| .await | ||
| .map(|(batch, _)| closed + batch); |
There was a problem hiding this comment.
🟡 Timer failure masks an incomplete store close
When sleep_until fails with a held store, close_idle_for_shutdown returns success after one final scan. The held store remains mounted without its shutdown checkpoint, and the timer failure disappears.
Learn more
A held runtime needs the polling window to release its lease before it can close. sleep_until returns an error if it cannot admit or join the native timer. Here that failure triggers an immediate final scan, but the scan's successful return overwrites the timer error. If the lease has not released, shutdown succeeds without closing the store or truncating its WAL.
Example: A runtime holds one client lease, and the native timer refuses admission on the first poll. The final scan finds the same lease, closes nothing, and returns Ok(0) instead of reporting the timer failure.
Recommended fix: Preserve the sleep_until error as a typed registry failure; optionally perform the final scan to close newly idle stores, but propagate the timer failure rather than replacing it with the scan result.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Fixes #4138.
Summary
Under load, a gracefully stopped daemon left WAL files behind.
store_shutdown_checkpoint_test::graceful_stop_truncates_profile_and_project_store_walsthen failed: 3,122,992 bytes ofglobal.db-wal, deterministic in the gate. On a clean origin/master the test failed 8 of 8 runs while a second copy of the test ran beside it. The daemon log shows the cause. Atstore_close_start,registry::closereported "store runtime still held at shutdown; its WAL is not truncated" forglobal.db(2 client leases), the project store (2 client leases and 1 graph lease) andsessions.db(3 client leases). The owners had joined, but the tasks holding their leases were still unwinding, and the close scanned once and gave up.crates/tracedecay-runtime-core/src/shard_runtime/registry/close.rs).close_idle_for_shutdownstill closes every idle runtime at once and concurrently. It then rescans the held runtimes every 10 ms and closes each one as its holders let go, for at most 3 s. The store-close reserve is 12 s, so this leaves most of it for the checkpoints. A runtime still held at the limit is logged with its blocker kinds, profile pins and graph leases, as before. This is an interim remedy; the step 7 storage executor owns close (docs/plans/storage/README.md). The project server lifecycle file is not touched.tracedecay_dashboard_api::cloud::fetch_latest_versionanswersNonewhenTRACEDECAY_NO_RELEASE_CHECKis set. The hermetic child environment (tests/support/isolated_profile.rs) sets it, so a spawned test daemon's advisory version check no longer calls the GitHub API. The daemon log had shown it hitting the GitHub rate limit from inside the test.docs/plans/storage/README.md,decisions.tsv):Test plan
graceful_stop_truncates_profile_and_project_store_walsruns with--runs_per_test=8, in parallel with the same run on origin/master (load-reproscript). With the fix it passed 8 of 8 in each of four rounds, while master failed 8, 7, 2 and 2 of 8 in the same rounds.shutdown_close_waits_for_a_lease_released_while_it_runs(runtime-core) releases a client lease 50 ms after close starts and asserts the runtime closes in that same call. The mutation above fails it (closed left 0, right 1).shutdown_close_drains_every_idle_runtime_concurrentlystill passes, so idle runtimes do not wait behind held ones.bazel test --config=clippyfor runtime-core and store-runtimeunit_test, dashboard-api, and//crates/tracedecay:daemon_suite(76 of 77 passed). The one failure,capacity_retirement_completes_while_a_maintenance_tick_runs, is a known flake in the gate's flaky log since 2026-10-15. It passes 3 of 3 alone.rustfmt_check_2024for runtime-core and dashboard-api