Repository navigation
Engine: settle live runs together on shutdown - #792
Open
RemingtonWilcox wants to merge 1 commit into
Open
RemingtonWilcox wants to merge 1 commit into
RemingtonWilcox wants to merge 1 commit into
Conversation
Engine shutdown interrupted live runs one at a time, and each interrupt waits for its run to settle, so stopping an engine with N live turns took about 3 s per turn. Interrupt them concurrently so it costs one bounded settle in total. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5 of 7 tasks
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.
Summary
Sessions::shutdowninterrupted live runs one after another, and each interrupt waits for its run to settle. A run whose harness doesn't wind down settles at the engine's 3 s interrupt deadline, so stopping an engine with N live turns took about 3 s per turn.join_all, asDocHost::shutdown_workersalready does), so shutdown costs one bounded settle in total however many turns are live. Each run still settles the same way: streaming entries are stampedabortedand the journal is closed.zeron headlessstopping (Ctrl-C,StopEngine,zeron daemon stop), and the desktop's runtime switch, which gives the daemon 10 s to stop. With 4 or more live turns that switch used to time out with "Could not stop the remote engine".Worth a close look
main, closing the window with live turns exits in about 0.3 s. GPUI giveson_app_quithandlers 200 ms (SHUTDOWN_TIMEOUT), logstimed out waiting on app_will_quit, and drops the task. That drop aborts the in-process engine shutdown throughgpui_tokio. Process exit then drops the Tokio runtime. I measured this with 3 live mock turns, and again with 3 turns whose agent CLI was a stand-in process that never writes or exits. In both cases the process exited and no agent processes were left behind. So the headed quit never waits for live turns: it doesn't hang, but it also doesn't finish the graceful drain (aborted stamps, final snapshot flush). Recovery on the next boot covers that today. I left it alone because it's a separate design question.EngineCore::shutdownandInProcessEngine::shutdownand found none without a bound that the quit path could block on, so this PR adds no extra timeout.tokio::fs::File, so their reads run on the blocking pool. If something kept an agent child alive past runtime shutdown, the process could linger. With the stand-in agent, the run tasks'Childdrop terminated the job and nothing lingered.Test plan
e2e::shutdown_settles_live_runs_together: 3 live runs whose harness ignores the interrupt;EngineCore::shutdownmust finish in under 6 s and stamp all threeaborted. Passes in about 3.3 s. Without the fix it fails at about 9.4 s (checked 4 times).cargo test -p zeron-engine --test e2eon Windows: everything else passes, exceptgenerated_image_is_materialized_before_publication_and_survives_reopen. That test also fails on unmodifiedmainhere.wrong_id_respond_is_rejected_and_correct_answer_still_resumes,harness_emitted_input_twin_is_dropped_and_answer_resumesandinterrupt_stamps_streaming_entry_abortedfailed now and then in full parallel runs, both with and without this change, and pass when run alone.cargo test -p zeron-engine --lib sessionsZERON_MOCK_DELAY_MS=1000,ZERON_MOCK_REPEAT=50), live turns started over IPC:zeron headless+StopEngine, 3 turns: 9.65 s before, 3.63 s afterui-testsworkflow): https://github.com/RemingtonWilcox/zeron/actions/runs/37240703827Screenshots
None: no visible change.
🤖 Generated with Claude Code