perf: fan out sidecar bring-up and collapse redundant podman calls#112
Merged
Conversation
The deferred efficiency items from task 0079's review; none change observable behavior (same tools, same deterministic label-collision errors, same session teardown semantics). Sidecar bring-up. setup_sidecars_and_network ran image-ensure, label read, container start, and bootstrap fully serially per sidecar. It is now three phases: ensure each distinct image and read its labels concurrently (join_all -- the codebase's fan-out idiom, which preserves input order and, unlike a JoinSet, needs no 'static bound on the borrowed config/transcript/plan), then merge labels and decide starts serially in plan.sidecars name order, then start containers concurrently. Keeping the merge pass name-ordered is what preserves the deterministic first-writer-wins collision errors. Two sidecars sharing one image now ensure and inspect it once; an image used only by anonymous sidecars is never label-inspected -- a correctness guard, since inspecting one could newly fail a session on a malformed org.outrig.mcp label the serial path never read. A deduplicated image feeds many sidecars but CliError is not Clone, so a failure is kept as its Display text and re-wrapped per consumer through io::Error (transparent) rather than OutrigError::Configuration, which would prepend "configuration: ". Only Display and an exit of 1 are observable, so the exact text -- not the variant -- is the contract. Session watcher. The watcher held one podman wait child for the primary plus one per sidecar (1 + N processes; podman wait given several names waits for all of them, so it cannot batch). It is now a single podman events --filter event=died --filter label=org.outrig.session stream per session. --since <started_at> replays a death that lands in the arm window; a shared-list membership gate on the sidecar-death line suppresses the replay of setup-time deaths of warn-dropped sidecars, which were never registered. The reader stops after the primary dies (so the reap's own sidecar deaths are not logged as spontaneous) and, on unexpected stream EOF, degrades to today's single-container "no auto-reap" without cancelling. register_sidecar is push-only now that the label-filtered stream covers a /sidecar add; wait_for_container_exit stays for attach-mode outrig mcp. outrig clean. The record walk spawned one podman inspect per aged session while the label sweep already listed every labeled container. Both now read one unfiltered podman ps -a: it yields the labeled stray rows (unchanged classify_strays input) and the set of all running container names, and the record walk answers by name from that set. Unfiltered because a record's primary can be outrig-unlabeled -- not only pre-label sessions but every outrig mcp --attach session, which records the borrowed container's name -- and a label-filtered listing would miss those. The stray podman rm -f calls are coalesced into one invocation. The remaining 0079 item -- overlapping sidecar image-ensure with the primary's bootstrap -- is deferred to plan/next; its win is bounded by a ~100ms bootstrap against restructuring the primary abort path.
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.
The deferred efficiency items from task 0079's review; none change observable behavior (same tools, same deterministic label-collision errors, same session teardown semantics).
Sidecar bring-up. setup_sidecars_and_network ran image-ensure, label read, container start, and bootstrap fully serially per sidecar. It is now three phases: ensure each distinct image and read its labels concurrently (join_all -- the codebase's fan-out idiom, which preserves input order and, unlike a JoinSet, needs no 'static bound on the borrowed config/transcript/plan), then merge labels and decide starts serially in plan.sidecars name order, then start containers concurrently. Keeping the merge pass name-ordered is what preserves the deterministic first-writer-wins collision errors. Two sidecars sharing one image now ensure and inspect it once; an image used only by anonymous sidecars is never label-inspected -- a correctness guard, since inspecting one could newly fail a session on a malformed org.outrig.mcp label the serial path never read.
A deduplicated image feeds many sidecars but CliError is not Clone, so a failure is kept as its Display text and re-wrapped per consumer through io::Error (transparent) rather than OutrigError::Configuration, which would prepend "configuration: ". Only Display and an exit of 1 are observable, so the exact text -- not the variant -- is the contract.
Session watcher. The watcher held one podman wait child for the primary plus one per sidecar (1 + N processes; podman wait given several names waits for all of them, so it cannot batch). It is now a single podman events --filter event=died --filter label=org.outrig.session stream per session. --since <started_at> replays a death that lands in the arm window; a shared-list membership gate on the sidecar-death line suppresses the replay of setup-time deaths of warn-dropped sidecars, which were never registered. The reader stops after the primary dies (so the reap's own sidecar deaths are not logged as spontaneous) and, on unexpected stream EOF, degrades to today's single-container "no auto-reap" without cancelling. register_sidecar is push-only now that the label-filtered stream covers a /sidecar add; wait_for_container_exit stays for attach-mode outrig mcp.
outrig clean. The record walk spawned one podman inspect per aged session while the label sweep already listed every labeled container. Both now read one unfiltered podman ps -a: it yields the labeled stray rows (unchanged classify_strays input) and the set of all running container names, and the record walk answers by name from that set. Unfiltered because a record's primary can be outrig-unlabeled -- not only pre-label sessions but every outrig mcp --attach session, which records the borrowed container's name -- and a label-filtered listing would miss those. The stray podman rm -f calls are coalesced into one invocation.
The remaining 0079 item -- overlapping sidecar image-ensure with the primary's bootstrap -- is deferred to plan/next; its win is bounded by a ~100ms bootstrap against restructuring the primary abort path.
Summary
What does this change, and why?
Checklist
cargo fmt --all -- --checkpassescargo clippy --workspace --all-targets -- -D warningsis cleancargo test --workspacepassesdoc/updated if behavior changed (mdbook buildstill clean)Notes for reviewers
Anything worth calling out -- tricky bits, follow-ups, or intentionally out-of-scope items.