fix(client-cohttp-eio): interruptible export waits at shutdown - #5
Open
dijkstracula wants to merge 2 commits into
Open
fix(client-cohttp-eio): interruptible export waits at shutdown#5dijkstracula wants to merge 2 commits into
dijkstracula wants to merge 2 commits into
Conversation
…ing the backoff Shutdown could stall ~3s: Collector.remove_backend runs one tick before cleanup while [stop] is still false, and send_http's failed-export backoff was a plain Eio_unix.sleep, so that pre-cleanup tick blocked out the whole backoff before cleanup (which sets stop and would interrupt it) could run. Move the backoff out of the send path: - send_http is fail-fast and time-bounded (send_timeout_s); no export, and thus no tick, ever blocks in a backoff, so the pre-cleanup tick cannot stall. - the ticker owns the backoff as its inter-tick wait; both the cadence and the backoff wait race a one-shot [shutdown] promise, so shutdown interrupts either. - the ticker stays a non-daemon fiber so its in-flight final export is awaited, not cancelled mid-send; cleanup's forced flush is Cancel.protect'd. Needs no core Collector.remove_backend change and no fork_daemon. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The semgrep org ruleset rejects any action not pinned to a full-length commit SHA, so the fork's mutable refs (checkout@main, checkout@v4, setup-ocaml@v3, install-nix-action@v30, actions-gh-pages@v3) failed every job at startup. Pin each to the SHA main already uses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dijkstracula
force-pushed
the
nathan/otel-interruptible-shutdown
branch
from
August 11, 2026 16:45
f92a019 to
b54c883
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
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.
Problem
When an OTLP collector cannot be reached during a traced run, exiting the process takes several seconds. Two things combine:
Collector.remove_backendcallsB.tick()before callingB.cleanup(). As a result, during shutdown an export withforce:falseruns while the flagstopis stillfalse;stopis only set incleanup.send_httpwaits for 3 seconds (Eio_unix.sleep 3.) when an export fails.Because this failed POST blocks for the whole backoff period before
cleanupruns, the loop cannot finish. Changing the sleep to be interruptible alone doesn't solve it, because no broadcast occurs while the tick is blocked; the signal that would stop the loop lives incleanup, which happens after the blocked tick.Fix
Move the backoff out of the sending logic so that an export, and thus a tick, never blocks on it:
send_httpnow fails fast and has a time limit (send_timeout_s). If it fails, it does not sleep. The timeout also limits a black-hole endpoint (accepts the connection but never replies), which before had no timeout.shutdownpromise (wait_or_shutdown). This means shutdown can interrupt them immediately.cleanupis wrapped withCancel.protect.No change to core
Collector.remove_backend, and no daemon fiber required.Validation
Measured with a standalone harness and mock collector for three cases: refused, reachable, and black‑hole:
ade04b2)cleanup(force:true)send_timeout_sRelationship to #4
This approach is an alternative to #4. While #4 removes the backoff and daemonizes the ticker, our fix keeps the backoff but makes every wait interruptible. Therefore we don't need a daemon or changes to final batch flushing. The shutdown latency for connection‑refused remains the same, and we also bound the black‑hole case.
Note: Because
remove_backendruns a tick before cleanup in core, this stall also affectsmain, upstream, and the ocurl/lwt backends. We plan an upstream report and PR as a follow-up.🤖 Generated with Claude Code