fix(daemon): keep runtimeless drops retiring until the drain confirms - #651
Conversation
A last-alias drop without a tokio runtime treated inability-to-await as a successful retirement and vacated the exact-store entry while retained producer frontends still held the shared core open, so a replacement mount could start a second producer for the same store. The entry now stays Stopping with a deferred drain; the next mount attempt on a live runtime starts the real core drain, and only its confirmed outcome releases (or fails) the entry.
|
4757f6a
into
codex/tracedecay-total-redesign-plan
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f61afbd024
ℹ️ 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".
| *drain = StoreObservabilityDrainV1::InFlight; | ||
| self.spawn_retirement_drain(&runtime, Arc::clone(core)); |
There was a problem hiding this comment.
Keep deferred drains retryable after task cancellation
When the first mount after a runtimeless drop runs on a short-lived or shutting-down Tokio runtime, this permanently changes the entry to InFlight before spawning a fire-and-forget drain. If that runtime cancels the task before finish_retirement, subsequent mounts continue returning Retiring and never restart the drain, wedging the store for the registry's lifetime. Retain a retryable state or track task cancellation through settlement.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
Summary
Resolves the unresolved Codex P2 finding on #650: "Keep runtimeless drops retiring until the owner closes."
The runtimeless-drop path shipped in #650 called
finish_retirement(&core, true)when the last alias handle was dropped outside a tokio runtime — treating inability-to-await as a successful retirement. That vacated the exact-store entry while a retained producer frontend (fromRegisteredObservabilityProducerV1::producer()) still held the shared producer core open, so the next mount could start a second producer for the same store while the first still accepted and wrote events — exactly the duplicate-producer bug the store-authority registry exists to prevent.Fix (
src/daemon/service/project_runtime/observability.rs)StoreObservabilityDrainV1 { InFlight, Deferred }on theStoppingstate records whether the core drain is actually running.Drop for RegisteredObservabilityProducerV1without a runtime now begins retirement asDeferredand never settles it: the entry staysStopping, refusing all mounts, and only warns.acquire_or_starthitting aDeferredretirement on a live runtime flips it toInFlight(under the registry lock, exactly once) and spawns the realcore.shutdown()drain — the mount itself is still refused withRetiring.finish_retirement: success vacates it, failure is remembered asFailed. Explicitshutdown()and runtime-present drops are unchanged (InFlight, sharedspawn_retirement_drain).Regression test (RED on pre-fix code)
runtimeless_last_alias_drop_keeps_the_store_retiring_until_the_drain_confirms: mounts a producer, retains the frontend fromproducer(), drops the last alias handle on astd::thread(no tokio runtime), proves the retained frontend still enqueues, then attempts an overlap mount whose start closure panics if reached.On the pre-fix code (
baa5cb8d9) the test fails with:i.e. the vacated entry let the registry invoke the start closure for a duplicate producer while the retained frontend was still live. Post-fix the overlap mount is refused
Retiring, the deferred drain then confirms (retained frontend observesobservability_producer_closed), and a replacement mounts and shuts down cleanly.Receipts
cargo test --lib daemon::service::project_runtime::observability_tests— 10 passed, 0 failed (includes the new test)cargo test -p tracedecay-usecases --lib observability— 70 passed, 0 failedcargo clippy -p tracedecay --lib --tests -- -D warnings— no diagnostics in touched files; remaining errors are pre-existing insrc/daemon/code_index_scheduler*(peer lane, unchanged here)cargo fmtclean on touched files