Repository navigation
Trace writer lifetimes and yield replication verdict pages - #6323
Draft
schickling-assistant wants to merge 2 commits into
Draft
schickling-assistant wants to merge 2 commits into
schickling-assistant wants to merge 2 commits into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Interactive terminal setup performs foreground store writes. Existing traces did not expose writer-acquire versus hold time for individual operations, and replication verdict pages still borrowed the foreground writer even though the scheduler already supports background loans.
Goal
Make writer contention attributable and let interactive foreground work overtake queued replication verdict pages without changing authority checks or introducing another scheduler.
Decisions
Two signed commits: request-parented writer lifetime instrumentation, then verdict-page classification and regression tests.
st.db.writerchild spans withwriter_wait_ms,writer_hold_ms,queue, and static caller location. These use a sampled request context without the opt-in storage-phase flag. Existing metrics and their cardinality are unchanged.Verification
One final-head gate-slot ticket is in progress:
cargo test --locked -p smallclaimsandcargo clippy --locked -p smallclaims --all-targets -- -D warnings. Results pending; no pass claimed.New deterministic coverage checks foreground batched progress during a real long verdict loop, trust-root changes between bounded loans before activation, and request-parented span fields/lifetime through finalization. No daemon restart or provider PTY input was used. Local pre-push evidence execution was disabled to prevent duplicate Rust admission; no CI dispatch requested.
Complexity
Reuse the existing scheduler, writer wait state, and context bridge. No second scheduler, busy-rejection path, or cancellation protocol.
Concerns
Priority is a dispatch-turn bound, not a latency guarantee: an active envelope, commit, and finalization remain non-preemptible. Concurrent long replication holds were observed, but the exact terminal timeout's writer wait remains unproven because its span had no writer attributes. The observed 3081 ms admission-loan hold from the 21:35Z trace is NOT fixed by this PR: that admission loan is already background. This PR makes the next reproduction name the holder site and distinguish writer wait from hold time. It does not claim to eliminate every attach stall.
Friction & bottlenecks
Existing telemetry lacked per-operation writer wait/hold attribution unless optional profiling paths were active. Instrumentation is trace-only and caller locations are not new metric labels.
Follow-ups
Use the new hold/caller attribution to identify any remaining non-preemptible long operation before changing chunking. No chunking changes are included here.
References
Related diagnostics-only work: #6320. This PR is independently based on main.
Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimeagent_modelworktree