iris-ci: the socket read timeout must follow the caller's deadline - #67
Merged
Merged
Conversation
mach-kernel
added a commit
to sgidevnet/irix-actions-runner
that referenced
this pull request
Jul 31, 2026
This reverts commit db304ab. The panic I blamed was a misread: make test was not killing the guest, iris-ci was giving up on the socket at 300s while the guest kept working. Fixed in techomancer/iris#67 and the worker image carries it now.
mach-kernel
marked this pull request as ready for review
July 31, 2026 13:52
Owner
|
hey it looks good but few issues. |
send_tcp and send_unix pinned the client read timeout at 300s while
--timeout was passed only to the server as wait-serial's timeout_ms. Any
guest command over five minutes therefore failed with
iris-ci: connect /tmp/iris.sock: Resource temporarily unavailable (os error 11)
while the guest carried on and finished the work, so the console showed no
error and --timeout looked like it did nothing.
Reproducer, on a restored guest with nothing else on the socket:
iris-ci run --shell sh --timeout 900 'sleep 400; echo LONG_OK'
Before: 5m14 and 5m29 across two runs, both EAGAIN. After: LONG_OK.
sleep 120 succeeded either way, which is what kept this hidden.
300s stays the default for commands that answer promptly. The three
wait-serial sites that carry a caller deadline derive the read timeout from
it, plus 30s so the server's own timeout fires first and names the pattern
it missed. The two fixed-deadline sites, at 5000 and 8000 ms, go through
read_timeout_for as well; they were already correct against the old default,
this makes the invariant hold by construction.
Every io error funnelled into Error::Connection and printed as "connect
<socket>", so a read deadline that expired mid-command reported as a failure
to connect and sent the reader to the wrong end. WouldBlock and TimedOut now
say so. That message is what hid this bug: the socket was always fine.
mach-kernel
force-pushed
the
fix/iris-ci-read-timeout
branch
from
August 1, 2026 18:19
34155a2 to
d53bb99
Compare
Contributor
Author
|
Sorted |
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.
send_tcpandsend_unixpinned the client read timeout at 300s, while--timeoutreached only the server aswait-serial'stimeout_ms. Any guest command over five minutes failed withconnect: Resource temporarily unavailable (os error 11)while the guest carried on and finished the work, so the console showed no error and--timeoutlooked like it did nothing.Changes
send_untiltakes the read timeout the caller needs;sendkeeps 300s for commands that answer promptly.wait-serialsites that carry a caller deadline derive from it:cmd_serial_wait(:527),run_capture(:591), and the deadline-based waiter (:688). Nothing else changes behaviour.READ_TIMEOUT_GRACEis 30s on top of the guest deadline, so the server's own timeout fires first and names the pattern it missed rather than the client bailing with a connection error.send_tcptakes the same parameter for symmetry. TCP mode is unexercised here.Error::Connectionand printed asconnect <socket>, so an expired read deadline reported as a failure to connect.WouldBlockandTimedOutnow say so. That message is what hid this bug: the socket was always fine.wait-serialsites (Password:at 5000 ms, the shell sentinel at 8000 ms) route throughread_timeout_foras well. They were already correct against the old default; this makes the invariant hold by construction.Coverage / needs eyes
iris-ci run --shell sh --timeout 900 'sleep 400; echo LONG_OK'. Before: 5m14 and 5m29 across two runs, bothEAGAIN. After:LONG_OK.sleep 120succeeded throughout at 2m04, which is what kept this hidden. Anything short works.cargo build --release --bin iris-ci --features chd,lightning,rex-jitclean.send_tcptakes the same change and I have not run it.sgidevnet/irix-actions-runnerruns each CI job as oneiris-ci runwith--timeout 14000. Every job step over five minutes died, the container exited 1, and the runner reported no step conclusion because the guest was never asked. A 20m34s job passes with this in.validateandpull/pushhave no--timeoutat all and can exceed 300s on a large snapshot or a slow registry; andz85c30.rs:1170rescans the whole accumulated buffer on every byte underwait_for, which is superlinear for a long command producing real output. My reproducer emits almost nothing, so it would not have caught the latter.read_timeout_foradds 30s to aDuration::from_millis(timeout_ms); a caller passing atimeout_msnearu64::MAXwould overflow in debug. Every current caller derives it from a--timeoutin seconds.