fix(store-runtime): cancel store opens at daemon shutdown - #2644
Conversation
A daemon stopped while a project open was inside its session-store mount waited for the mount to finish: the registry open ran detached, the schema install could not be interrupted, and the shutdown close skipped runtimes still opening. Under CPU starvation the mount outlived the 2 s cooperative drain, the open was aborted, and the process missed its exit. The store runtime registry now owns a shutdown cancellation. The project open shutdown owner cancels it for every mounted session registry. New opens are refused with a typed OpenCancelled; an in-flight open stops before resolve and before publish, and one that finishes publishing after the cancel closes its unpublished runtime instead of publishing it. The registered schema install, both at initialization and at daemon attach, checks the token before every statement of its admission transaction and rolls back with a typed store_open_cancelled, so a store is left either with its prior schema or fully installed. The shutdown close cancels and joins in-flight opens before it closes idle runtimes, so none is skipped.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f17d16e7ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Some(registry) = entry.registry.get() { | ||
| registry.cancel_store_opens_for_shutdown(); |
There was a problem hiding this comment.
When shutdown races with the first profile access, this OnceCell is still empty even though DaemonSessionRuntimeRegistryV1::open_with_session_maintenance has already created its StoreRuntimeRegistry and is awaiting the profile store/schema open. get() therefore skips precisely that in-flight registry, while the project-open/bootstrap future has no cancellation-aware select around initialization; shutdown can still wait for the full mount and exceed its cooperative deadline. Keep a cancellation authority in the registry entry before starting the OnceCell initializer, or otherwise propagate shutdown cancellation into an initializer whose cell is not yet populated.
Useful? React with 👍 / 👎.
…ssues-shutdown # Conflicts: # crates/tracedecay-global-db/src/registered.rs # crates/tracedecay-global-db/src/schema_stages.rs # crates/tracedecay-store-runtime/src/session_registry/maintenance.rs
1 participant
Refs #2430
Root cause
A daemon stopped while an admitted project open was inside its session-store mount waited for that mount to finish (measured in #2430 by the master-reds lane on #2612/b896d9f8db). The mount had no safe points:
StoreRuntimeRegistry::begin_or_join_openruns resolve + publish (including the Initialize schema install) in a detached task that never observed shutdown,attach_registeredthen ran the whole registered-schema admission (ensure_attached_registered_schema, the 127 ms of the 165 ms mount unthrottled) without a cancellation point, andclose_idle_for_shutdownskipped runtimes stillOpening. Under CPU starvation the mount outlived the 2 s cooperative drain, the open was aborted, and an aborted open could not close its stores exactly.Change
StoreRuntimeRegistryowns a shutdown cancellation (cancel_opens_for_shutdown). New opens are refused with typedStoreRuntimeRegistryFailure::OpenCancelled; an in-flight open stops before resolve and before publish; an open that finishes publishing after the cancel closes its unpublished runtime and failsOpenCancelledinstead of publishing.RegisteredSchemaInstallationV1::cancellation, daemon attach viaadmit_and_attach_for_daemon): before classification, before the admission transaction, and before every statement of it. A cancel rolls the transaction back and fails with typedTraceDecayError::store_open_cancelled(store_open_cancelled, retryable), so the store keeps exactly its prior schema; a cancel after commit lets the idempotent index builds and validation finish, so it is never half-installed. An Initialize-mode cancel also aborts the prospective file.close_idle_for_shutdowncancels and joins in-flight opens before it scans, so none is skipped or publishes behind the close.project_openshutdown owner cancels store opens for every mounted session registry before it drains project opens. No deadline, drain window, or test bound changed.Evidence
Fail-before (fix neutralized:
cancel_opens_for_shutdownmade a no-op and both schema checkpoints disabled; on origin/master the new APIs do not exist):Pass-after on this branch (rebased on 6bc1138): both pass. The schema test sweeps the cancel across every 20th statement boundary of a real daemon attach on an empty existing profile-sessions store: every cancel before the commit ends
store_open_cancelledwith zero schema objects, every later one ends with the exact full object set, and a reopen after each succeeds with the full set.Focused suites (non-zero counts):
tracedecay-global-db --lib386 passed;tracedecay-runtime-core --lib454 passed;tracedecay-store-runtime --lib122 passed (1 ignored, pre-existing);tracedecay --lib -- daemon::store_runtime_tests daemon::tests::bootstrap daemon::engine project_open138 passed;core_cli_suite -- tool_daemon_test41 passed (includesdaemon_sigterm_exits_while_authenticated_project_client_is_connectedanddaemon_sigterm_stops_an_in_flight_open_at_the_next_store_boundary).cargo clippy -p tracedecay-domain -p tracedecay-runtime-core -p tracedecay-global-db -p tracedecay-store-runtime -p tracedecay --all-targets --features tracedecay/test-helpers,tracedecay/test-transport -- -D warningsclean;cargo fmt --all -- --checkclean.Throttled-SIGTERM loop (the #2430 harness, real debug CLI)
daemon_sigterm_exits_while_authenticated_project_client_is_connecteddriven through aTRACEDECAY_TEST_BINwrapper that runs every daemon undersystemd-run --user --scope -p MemoryMax=6G -p MemorySwapMax=1Gand setsCPUQuotaon the test's daemon once it logsevent=daemon_ready. Same test binary for both sides; "before" is the master daemon (10b4ff2), "after" this branch.project_server_warmup outcome=cancelledFinal binary, one run (
rebased-q5-1):Left open (why this is
Refs, notFixes)After this change no run spends the cooperative window inside the open, but at ≤5% CPU the test still misses 3 s in some runs on both sides at the same rate: the rest of the shutdown sequence alone exceeds it. In those runs every phase advances in ~100 ms CFS quota periods and the terminal
memory_graph_reconciliationstore close takes 0.8–1.4 s (e.g. before-q3-2: project_open 694 ms, store close 1399 ms; after-q3-6: project_open 297 ms, store close 1102 ms). That store-close cost is a separate measured target and is not changed here.ripwire:
--edit-checkonadmit_and_attach_for_daemon,ensure_attached_registered_schema,install_and_commit_registered_schema,install_from_authorized_connection,close_idle_for_shutdown: all callers compatible.--quality-delta=f5bd745eed..HEAD: theclose_idle_for_shutdowncomplexity/verbosity regression it flagged was fixed by moving the join intocancel_and_join_opens_for_shutdown; the remaining gating rows are 18–47-tokenif cancelled { return Err(..) } Ok(())shape matches against unrelated crates and a test row-collection loop, and the dead-code rows are cross-crate callers ripwire does not resolve.