Repository navigation
pilotctl: --reuse-conn after a retry, message_id on failure, dgram codes, bench fails on a partial echo - #520
Merged
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if !jsonOutput { | ||
| fmt.Printf(" Sent: %s in %v (%.1f MB/s)\n", formatBytes(uint64(sent)), sendDuration.Round(time.Millisecond), sendThroughput) | ||
| fmt.Printf(" Echoed: %s in %v (%.1f MB/s round-trip)\n", formatBytes(uint64(recvTotal)), totalDuration.Round(time.Millisecond), totalThroughput) | ||
| fmt.Printf(" Echoed: %s in %v (%.1f MB/s round-trip)\n", formatBytes(uint64(received)), totalDuration.Round(time.Millisecond), totalThroughput) |
| // send-message --count carries each message's result. | ||
| if !complete { | ||
| fatalResults = []map[string]interface{}{stats} | ||
| got := fmt.Sprintf("%s of %s", formatBytes(uint64(received)), formatBytes(uint64(totalSize))) |
| // send-message --count carries each message's result. | ||
| if !complete { | ||
| fatalResults = []map[string]interface{}{stats} | ||
| got := fmt.Sprintf("%s of %s", formatBytes(uint64(received)), formatBytes(uint64(totalSize))) |
TeoSlayer
force-pushed
the
fix/pilotctl-followups
branch
2 times, most recently
from
October 8, 2026 09:50
f893bc0 to
6db1a93
Compare
A lost ack means the connection is gone: no read deadline is set, so the ack read failed because the connection ended. The --reuse-conn loop still wrote the next messages into it, and since driver.Conn.Write only notices a local close those writes "succeeded" and were dropped. Each later message then lost its ack and was retried on a new connection, which the retry closed straight away: --count 3 with the first ack lost made 4 dials, every message showed retried=true, and with --no-resend all 3 of 3 failed although only one ack was lost. sendOne now takes the connection over and hands back the one to continue on: the retry's connection (kept open) when the first ack was lost, nil when neither can carry another message, in which case the loop dials a new one. reused is true only when that connection carried an earlier message. A later message whose new connection cannot be dialled is recorded as failed and the run goes on. The fake data-exchange receiver in the tests now discards what is written to a connection it closed, as a real daemon does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The single-send exits (send failed, not acknowledged, refused by the receiver) did not set fatalResults, so the --json error had no message_id and the caller could not look for the message in the receiver's inbox or for a reply naming it. They now put the message's result in the envelope's "results", as a --count run already does. The refusal exit used fatalCode, which never carries results; it is now a fatalHint with a hint. fatalResults is a package global, so the test resets it first: a value left by another test would pass for this one's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cmdDgram mapped every refusal from the daemon to connection_failed,
which the error-code list calls "may retry". A port the network's policy
forbids or a datagram too large to send fails the same way every time.
dgramErrorCode now maps:
- port policy ("not allowed by network %d policy") -> permission_denied
(added to the context error_codes list);
- payload too large, the socket's "message too long", "ipc frame too
large", the broadcast-address refusal -> invalid_argument;
- a resolve the registry answers "not found" -> not_found;
- driver.ErrConfirmQueueTimeout (nothing was sent) -> timeout, like
ErrConfirmTimeout already was;
- everything else -> connection_failed.
A --data over 65535 bytes (a packet's payload length is 16 bits) is
refused before the daemon is contacted; the port and --data checks now
run before connecting. dgram --help gives the limit and the codes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bench printed "status":"ok" and exited 0 when the echo stopped short: with the echo cut off after 2 KiB, --timeout 300ms reported 4096 of 10485 bytes received as a success. A short echo now exits 1 with code timeout (the wait for the echo hit --timeout) or connection_failed (the echo stream ended early), and the partial figures go into the error envelope through fatalResults, as a failed send-message --count's results do. A complete run adds "complete": true. total_mbps was the full size over the elapsed time; it is now the bytes that came back. The received count was read without synchronisation while the reader goroutine could still be writing it; it is an atomic.Int64, read once after the wait. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The send-message changelog entry said a receiver that predates message IDs (v1.13.9 and older) costs "one extra exchange". It stores nothing for the tagged frame, so Client.Send sends the whole message again without the ID: up to 64 MiB a second time, and both copies again after a lost-ack retry. The changelog, the send-message help and the sendOne comments now say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Without --reuse-conn, a --count run whose later dial failed exited at once with no results: the message IDs of the messages already delivered were lost, and a caller sending them all again duplicated them. The dial failure is now recorded against its message and the run goes on, as with --reuse-conn. - dgram: the daemon refuses a payload over 65535 bytes, but the datagram and its headers must fit one UDP packet, so somewhat less fits in practice. The help and the refusal say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer
force-pushed
the
fix/pilotctl-followups
branch
from
October 8, 2026 10:04
6db1a93 to
6a79647
Compare
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
2 participants
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.
pilotctl follow-ups from reviews of v1.17.0 changes, each confirmed by a probe on main.
--count --reuse-connafter a lost ack.reused: true. The probe made 4 dials with every messageretried; with--no-resend, 3 of 3 failed although one ack was lost.reusedis true only when that connection carried an earlier message. A failed redial is recorded per message.message_idin the error envelope (results), like--count.dgramexit codes follow the daemon's reason. Previously everything wasconnection_failed("may retry").permission_deniedinvalid_argumentnot_foundtimeoutconnection_failed--dataover 65535 bytes is refused before contacting the daemon.connection_failedwill see these new codes.benchfails on a partial echo."status":"ok"with 4 KiB of 10 KiB back. A short echo now exits 1 (timeoutorconnection_failed), with the partial figures inresults. A complete run adds"complete": true.total_mbpscounts what came back, and a data race on the received count is fixed.Each test fails without its fix.
go test -race ./cmd/pilotctlpasses, and the CLI reference has no change.🤖 Generated with Claude Code