[runner] Mark logs finished only once the executor has finished - #4280
Merged
Merged
Conversation
Previously, `Server.stop()` set WaitLogsFinished as soon as `/api/stop`
arrived. `/api/pull` treats the first request it serves in that state as
the final one and closes `pullDoneCh` -- the barrier that keeps the
runner alive until the server has fetched the job's last logs and state.
The job is still stopping at that point. The command has up to
`killDelay` before it is killed, so the state being served is not final,
and the barrier is satisfied before the state it is meant to protect
exists:
08:38:39 /api/stop
08:38:41 /api/pull <- closes pullDoneCh here
08:38:51 Job state changed new=terminated
08:38:51 Job finished, shutting down
08:38:51 Logs streaming finished endpoint=/api/pull <- 92us later
The runner exited 64ms after computing the terminal state, without ever
serving a pull that carried it. The server's next poll found nothing
listening, reported the instance unreachable, and terminated the job on
the disconnect timeout two minutes later -- discarding the state the
runner had worked out.
Now `stop()` only cancels the run. The executor sets WaitLogsFinished
from its own defer when `Run` returns, after the terminal job state has
been recorded, so the first pull to observe it carries that state and
the runner exits only once the server has taken it.
`HasMore` is derived from the same state and was likewise reported as
false for the whole stop window. Nothing observed it: the server's
`PullResponse` has no such field.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Previously,
Server.stop()set WaitLogsFinished as soon as/api/stoparrived./api/pulltreats the first request it serves in that state as the final one and closespullDoneCh-- the barrier that keeps the runner alive until the server has fetched the job's last logs and state.The job is still stopping at that point. The command has up to
killDelaybefore it is killed, so the state being served is not final, and the barrier is satisfied before the state it is meant to protect exists:The runner exited 64ms after computing the terminal state, without ever serving a pull that carried it. The server's next poll found nothing listening, reported the instance unreachable, and terminated the job on the disconnect timeout two minutes later -- discarding the state the runner had worked out.
Now
stop()only cancels the run. The executor sets WaitLogsFinished from its own defer whenRunreturns, after the terminal job state has been recorded, so the first pull to observe it carries that state and the runner exits only once the server has taken it.HasMoreis derived from the same state and was likewise reported as false for the whole stop window. Nothing observed it: the server'sPullResponsehas no such field.