Release 0.6.7 — four bugs the instruments called healthy - #21
Merged
Merged
Conversation
hackney 4.7.2 -> 4.7.4, h2 0.11.0 -> 0.12.0, webtransport 0.4.4 -> 0.4.5 and quic 1.8.0 -> 1.8.2 are the HTTP stack behind tracker announces and BEP 19 web seeds, so the bump is validated by the full local CI gate, not just a compile. dialyxir and ex_doc are dev-only. Co-authored-by: Cursor <cursoragent@cursor.com>
:add_peer_failed is emitted by add_peer/6 only after TCP connect and the
full BEP 3 handshake have completed -- we already hold the peer's id and
reserved bytes -- and means Swarm.add/4 could not start the peer
supervisor. That is the same class as :socket_handoff_failed, which was
moved out of the reachability signals in an earlier pass; the sibling one
step earlier on the same path was missed.
As a result it counted as a DialStats family failure, depressing the
measured v4/v6 yield that drives the per-family dial throttle, and wrote a
DialBackoff row toward @hard_fail_threshold. Live, one torrent held 61 of
its 62 known endpoints sticky-blocked with 4 peers connected.
The catch-all also hid the outcome that actually produced it: peer
supervisors register under {peer_id, hash}, so a fake-peer farm handing one
id out from many IPs loses the race at start_child. That is
:already_connected, which every stage downstream already understood.
Co-authored-by: Cursor <cursoragent@cursor.com>
The line read 'request failure reason: <term>' with no info_hash and no
announce URL, which made the most actionable message in the log useless:
a tracker's own bencoded failure reason asks the user to re-add a torrent
it never named. Logging moves out of retry_interval_seconds/2, which is now
pure, into parallel_tracker_error/4, where the request ref has been
resolved back to its announce URL.
Two classification gaps went with it. HTTPoison surfaces hackney's connect
timeout as {:timeout, {:gen_statem, :call, [pid, :connect, 8000]}}, so the
connect-timeout case expected_tracker_failure_reason?/1 exists to cover
never matched -- 42 of 89 warnings in a 21-minute window. HTTP 4xx/5xx was
not classified at all, though a 403/404/521 from a public announce-list
entry is a dead tracker and BEP 12 already fails over tiers. Both are
expected now; a bencoded failure reason still warns.
Co-authored-by: Cursor <cursoragent@cursor.com>
…matched Every shape a tracker task is meant to return is matched by an earlier handle_info clause, so the catch-all fires only on something nobody wrote. It used to drop the reply silently; the previous commit made it audible as :unexpected_reply, which named the category and no evidence -- the same gap that commit set out to close. It now carries the term. Co-authored-by: Cursor <cursoragent@cursor.com>
…forever
`tracker.openbittorrent.com` has neither an A nor an AAAA record, and Hackney
raises `:error, :badarg` out of its connect path for a name with no addresses
rather than returning `:nxdomain`. `http_announce_bound/4` caught that and
returned `%Error{reason: :badarg}` with no `retry_in`, which falls into the
generic failure clause and gets the default retry interval — so the dead host
was re-announced every cycle forever, six times an hour across four torrents.
The UDP side has never had this problem: it resolves up front via
`resolve_hosts/1` and answers `retry_in: "never"`, which `PeerDiscovery.Announce`
uses to drop a tracker from the rotation for the session. That clause was written
for the defunct rarbg trackers; openbittorrent is the same species of host and
simply took a code path that could not reach it.
The `:badarg` catch now resolves the host itself and claims `{:nxdomain, host}`
with `retry_in: "never"` only when DNS genuinely has nothing. `:badarg` has other
possible sources and a permanent disable is too destructive to apply on a guess,
so anything else keeps the old opaque reason.
Found one hour after the announce-failure logging started naming the reason:
`reason=:badarg` is self-evidently not a network condition. It is the third
instance of one root pattern — a library wraps or replaces the error shape our
classification was written against, and the mismatch fails open into "retry
forever / warn forever" rather than into an error, so nothing signals that the
guard has gone dead.
Co-authored-by: Cursor <cursoragent@cursor.com>
…d sample
`Torrent.Model` differenced `downloaded` over its 5 s tick, but `downloaded`
advances only when a whole piece completes and verifies. The sample was therefore
quantized to piece size: at 55 KB/s with 1 MiB pieces a piece lands every ~19 s,
so three ticks in four read exactly 0.0. Live, two torrents at 39.7% and 52.0%
both reported 0 B/s while gaining a combined 196 KB/s, and `compute_eta/4` in the
WebUI saw `kbps <= 0` and answered `:infinity` for everything.
`download_rate/1` now averages `delta / elapsed` over a 60 s window — the same
measurement a human audit performs by summing `left` deltas — holds the last
published average while a window containing progress matures, and clamps by
`piece_length / elapsed` only when nothing has arrived, since were the torrent
still running that fast the first piece of the window would already have landed.
Nothing completed for 10 minutes reports 0.
Two narrower approaches were tried against live traffic first and are recorded in
the code comment, because each looks obviously right:
* An EMA over the quantized samples. Any time constant short enough to track a
fast torrent still collapses between a slow torrent's pieces — measured
decaying to 1e-39 — and one long enough for the slow torrent is uselessly
laggy for the fast one.
* Timing each arrival against the previous arrival. A piece completing inside a
single tick makes the measured interval ~5 s, a real burst rate but a bad
basis for a stall cutoff, so the readout alternated between the burst and 0.
The subtle one inside the current design: applying the silence ceiling *while* the
window had progress in it made a true 100 KB/s read as 11 KB/s, because the
ceiling shrinks as the window ages. Progress in the window has to disable it.
Measured before/after on the same live torrent: constantly 0.0 while gaining
~100 KB/s, to readings of 12-157 KB/s around a true 64 KB/s average. The residual
oscillation is inherent to a piece-granular source on a bursty swarm; a complete
fix needs byte-granular reporting from `Peer.Controller`, which already tracks
`downloaded_bytes` per connection.
Co-authored-by: Cursor <cursoragent@cursor.com>
`mark_bad`/`mark_query_failed` on an id that is not in the table are no-ops, and the assertions said so by comparing the result against a second `RoutingTables.new(@hash)`. The struct carries `last_changed_ms` from the monotonic clock, so under full-suite load the two constructions land in different milliseconds and the equality fails; it passed in isolation and failed once in ten full runs. The claim is that the call is a no-op, not that `new/1` is deterministic, so bind the table once and compare against that value. Co-authored-by: Cursor <cursoragent@cursor.com>
The peer-id prefix is derived from the package version (BEP 20), so this bump is visible on the wire as `ET0-6-7` and worth its own commit: every peer that sees us during this release identifies the build by it. The changelog entry groups the four fixes by what they had in common rather than by subsystem — each one was the engine misbehaving while its own instruments read normal, which is why all four survived so long. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
`Merkle.leaf_range_response_from_disk/7` documents `{:ok, [hash()]} | {:error,
term()}` and already handles a failed `:file.open/2` that way, but the per-leaf
reads underneath it pattern-matched `{:ok, block} = :file.pread(fd, offset,
size)`. An I/O error, or a file truncated between the stat that produced
`file_length` and the read, therefore raised `MatchError` out of the middle of a
function whose contract says it returns errors.
`HashServe` catches the raise and answers `hash_reject`, which is the correct
thing to put on the wire for a hash request we cannot serve (BEP 52) — so this was
low severity in the shipped path, and wrong everywhere else: any other caller got
an exception for an ordinary disk condition, and the reject was reached by
accident rather than by decision.
`read_leaf_cache/4` now short-circuits on the first unreadable leaf, since the
cache is only useful complete — every consumer does `Map.fetch!/2` on it.
One subtlety worth naming: `:file.pread/3` answers a bare `:eof`, not an error
tuple, when there is nothing at the offset. That is *not* the same as the
legitimate case of a leaf beyond the end of the file, which BEP 52's power-of-two
leaf padding makes routine and which hashes to the zero hash without reading at
all. The padded case is now a guard clause on `offset >= file_length`, so a bare
`:eof` from an actual read means `file_length` no longer describes the file and is
reported as a failure.
+2 tests: the truncated-file contract, and a whole-file read whose last block is
short, so the ragged tail the new guard touches stays covered.
Co-authored-by: Cursor <cursoragent@cursor.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
1 participant
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.
Four fixes plus the 0.6.7 release. They are grouped in the changelog by what they had in common rather than by subsystem: each one was the engine misbehaving while its own instruments read normal, which is why all four survived so long.
What changed
A tracker whose hostname no longer resolves is dropped for the session. Hackney raises
:badargout of its connect path for a name with no A and no AAAA record instead of returning:nxdomain, so the HTTP announce returned a reason with noretry_inand fell into the generic clause with the default retry interval.tracker.openbittorrent.com, long defunct, was re-announced six times an hour across four torrents. The UDP side never had this problem — it resolves up front and answersretry_in: "never", whichPeerDiscovery.Announcealready uses to drop a tracker from the rotation. The:badargcatch now resolves the host itself and only writes a tracker off when DNS genuinely has nothing; other sources of:badargkeep the old opaque reason, because a permanent disable is too destructive to apply on a guess.Announce failures name the torrent and the announce URL. The line read
request failure reason: <term>with neither, so the most actionable message in the log — a tracker's own bencoded reason asking the user to re-add a torrent — never said which torrent. Two classification gaps went with it: HTTPoison surfaces hackney's connect timeout as{:timeout, {:gen_statem, :call, [pid, :connect, 8000]}}, which the connect-timeout case never matched (42 of 89 warnings in a 21-minute window), and HTTP 4xx/5xx was not classified at all even though a 403 or 521 from a public announce-list entry is a dead tracker that BEP 12 already fails over.:add_peer_failedno longer counts as a peer being unreachable. It is emitted only after TCP connect and the full BEP 3 handshake succeed — we already hold the peer's id — and means the supervisor could not start, which is a local fault. It was depressing the per-family dial yield that drives the address-family throttle and writingDialBackoffrows toward the hard-fail threshold: one torrent held 61 of its 62 known endpoints sticky-blocked while connected to 4 peers. The catch-all also hid the outcome behind it: peer supervisors register under{peer_id, hash}, so a farm handing one id out from many IPs loses the race atstart_child, which is:already_connected.The download speed readout no longer reports 0 B/s on a moving torrent.
Torrent.Modeldifferenceddownloadedover its 5 s tick, but that counter advances only when a whole piece verifies, so the sample was quantized to piece size — at 55 KB/s with 1 MiB pieces three ticks in four read exactly0.0. Two live torrents reported 0 B/s while gaining a combined 196 KB/s, and the ETA derived from it was:infinitythroughout. The rate is now averaged over a 60 s window, held while a window containing progress matures, and clamped bypiece_length / elapsedonly when nothing has arrived at all.Dependencies: the HTTP stack behind tracker announces and BEP 19 web seeds moved up (hackney 4.7.2 to 4.7.4, h2 0.11.0 to 0.12.0, webtransport 0.4.4 to 0.4.5, quic 1.8.0 to 1.8.2), which is why this is validated by the full gate rather than a compile.
What is deliberately not fixed
The rate readout is improved, not cured, and the commit says so. The source is still piece-granular, so a bursty swarm still oscillates and a torrent slower than one piece per window reads 0 until its first piece lands. The real fix is byte-granular reporting from
Peer.Controller, which already tracksdownloaded_bytesper connection — that touches the hot receive path and is a separate decision.Two narrower approaches were tried against live traffic first and are recorded in the code comment because each looks obviously right: an EMA over the quantized samples (any time constant short enough for a fast torrent collapses between a slow one's pieces — measured decaying to
1e-39), and timing each arrival against the previous arrival (a piece completing inside one tick makes the interval ~5 s, so the readout alternated between a burst and 0). The subtle one inside the current design: applying the silence ceiling while the window held progress made a true 100 KB/s read as 11 KB/s, because the ceiling shrinks as the window ages.Verification
Every gate of
.github/workflows/build-and-publish.ymljobbuildwas run locally, in CI's order, before pushing. All 12 green:mix testmix dialyzermix credo --allmix format --check-formattedmix compile --warnings-as-errors(dev + test)mix sobelowmix hex.auditmix deps.get --check-locked(dev + test)mix deps.unlock --check-unusedscripts/compile-property-deps.shDeployed and verified live: the engine reports
0.6.7andET0-6-7is on the wire (the peer-id prefix is version-derived, BEP 20). Both previously stalled torrents resumed within 45 seconds at 17.5 and 52.4 KB/s with sane ETAs.Not reproducible locally, so not claimed green: the Trivy filesystem scan, the Codecov upload, and the SARIF upload to code scanning.