Skip to content

fix(client-cohttp-eio): don't block process exit on shutdown - #4

Open
dijkstracula wants to merge 2 commits into
forkfrom
nathan/skip-otel-backoff-on-shutdown
Open

fix(client-cohttp-eio): don't block process exit on shutdown#4
dijkstracula wants to merge 2 commits into
forkfrom
nathan/skip-otel-backoff-on-shutdown

Conversation

@dijkstracula

@dijkstracula dijkstracula commented Aug 6, 2026

Copy link
Copy Markdown

What

On a traced run (--trace) whose OTel collector is slow or unreachable, process
exit stalled for several seconds. This removes both causes.

  1. Drop the 3s export backoff. send_http did Eio_unix.sleep 3. after a
    failed POST. Because cleanup's forced flush also goes through send_http,
    that sleep ran at teardown and added ~3s to every exit. Removed; the batch
    machinery (batch_timeout, drain-on-pop) already paces retries, and the
    error report is gated on not (Atomic.get stop) so a failed shutdown export
    stays quiet rather than spamming.

  2. Daemonize the ticker. The 0.5s inter-tick Eio.Time.sleep was in a plain
    awaited fiber, so the switch waited it out at teardown (≤0.5s tail). It's now
    a fork_daemon, so the switch cancels the sleep at teardown. Safe because
    cleanup force-flushes the final batch before the switch returns.

Evidence

Fragment-scanner APM traces: Pro_core_CLI.main_exn is unchanged (~1.8s), while
the total dropped from ~5.9s (3s-sleep) to ~3.2s (no-sleep) — the delta was pure
shutdown tail.

On a traced run (--trace) when the OTel collector is slow or unreachable, the process exit took several seconds. The changes below fix both reasons.

  1. Remove the 3‑second export backoff.
    send_http used to call Eio_unix.sleep 3. after a failed POST. Because cleanup also calls send_http, that sleep happened during teardown and added about 3 seconds to every exit. It has been removed. The batch system (batch_timeout, drain‑on‑pop) already controls retry timing, and the error report is only sent when not (Atomic.get stop) so a failed shutdown export stays silent instead of spamming.

  2. Run the ticker as a daemon.
    The 0.5‑second pause between ticks used to be in a normal awaited fiber, so the switch waited for it during teardown (up to 0.5 seconds). Now it is started with fork_daemon, which lets the switch cancel the sleep at teardown. This is safe because cleanup force‑flushes the final batch before the switch returns.

Evidence

Fragment‑scanner APM traces show that Pro_core_CLI.main_exn time stayed about 1.8 seconds, while the total time dropped from roughly 5.9 seconds (with the 3‑second sleep) to about 3.2 seconds (without the sleep). The difference is just the shutdown tail.%

@dijkstracula
dijkstracula force-pushed the nathan/skip-otel-backoff-on-shutdown branch from fc60ad9 to 050649e Compare August 6, 2026 05:14
@dijkstracula dijkstracula changed the title fix(client-cohttp-eio): skip 3s error backoff during the shutdown flush fix(client-cohttp-eio): don't log or back off on export failure during the shutdown flush Aug 6, 2026
@dijkstracula
dijkstracula force-pushed the nathan/skip-otel-backoff-on-shutdown branch 2 times, most recently from 050649e to 1ecb66b Compare August 6, 2026 05:24
@dijkstracula dijkstracula changed the title fix(client-cohttp-eio): don't log or back off on export failure during the shutdown flush fix(client-cohttp-eio): drop the 3s error backoff that stalls shutdown Aug 6, 2026
@dijkstracula
dijkstracula requested a review from rhwlo August 6, 2026 05:25
@dijkstracula
dijkstracula force-pushed the nathan/skip-otel-backoff-on-shutdown branch 2 times, most recently from d2bbc3a to 6e8e472 Compare August 6, 2026 05:31
@dijkstracula
dijkstracula changed the base branch from main to fork August 6, 2026 05:31
@dijkstracula
dijkstracula requested a review from ajbt200128 August 6, 2026 16:07
@dijkstracula
dijkstracula force-pushed the nathan/skip-otel-backoff-on-shutdown branch from 6e8e472 to 0889e81 Compare August 6, 2026 16:30
@dijkstracula dijkstracula changed the title fix(client-cohttp-eio): drop the 3s error backoff that stalls shutdown fix(client-cohttp-eio): make the export backoff interruptible at shutdown Aug 6, 2026
@dijkstracula
dijkstracula force-pushed the nathan/skip-otel-backoff-on-shutdown branch 2 times, most recently from 142bbe3 to ade04b2 Compare August 6, 2026 17:15
On a traced run whose collector is slow or unreachable, process exit stalled for
seconds. Two independent causes, both removed:

1. send_http slept 3s after a failed POST as a retry throttle. That sleep also
   ran during cleanup's forced flush, adding ~3s to every exit. Drop it: the
   batch machinery already paces sends, and the error report is gated to
   non-shutdown so a failed teardown export stays quiet.

2. The tick fiber was a plain awaited fiber, so the switch waited out its 0.5s
   inter-tick sleep at teardown. Fork it as a daemon so the switch cancels that
   sleep instead. cleanup force-flushes the final batch before the switch
   returns, so cancellation can at worst drop a tick already mid-send.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dijkstracula dijkstracula changed the title fix(client-cohttp-eio): make the export backoff interruptible at shutdown fix(client-cohttp-eio): don't block process exit on shutdown Aug 7, 2026
@dijkstracula
dijkstracula force-pushed the nathan/skip-otel-backoff-on-shutdown branch from ade04b2 to 2da6919 Compare August 7, 2026 22:46
Comment thread .github/workflows/gh-pages.yml Outdated
The semgrep org only allows SHA-pinned actions, so the tag/branch pins
(checkout@v4/@main, setup-ocaml@v3, install-nix-action@v30, actions-gh-pages@v3)
failed every job at set-up. Pin each to a full commit SHA with the version in a
trailing comment.
@dijkstracula
dijkstracula force-pushed the nathan/skip-otel-backoff-on-shutdown branch from 090c28f to 7b1335c Compare August 10, 2026 16:18
malaverdiere pushed a commit to semgrep/semgrep that referenced this pull request Sep 4, 2026
…prietary#6864)

Bumps the `semgrep/ocaml-opentelemetry` pin `6cfa5f16` → `2da6919` to
pull in the
shutdown fix from **semgrep/ocaml-opentelemetry#4**.

On a traced run (`--trace`) whose collector is slow or unreachable, the
cohttp-eio exporter blocked process exit for several seconds. The fork
fix
removes both causes:

1. drops `send_http`'s 3s failed-POST backoff (which also ran during
`cleanup`'s
   forced flush), keeping the error report gated to non-shutdown; and
2. daemonizes the tick fiber so the switch cancels its 0.5s inter-tick
sleep at
teardown (`cleanup` force-flushes the final batch first, so no data loss
   beyond a tick already mid-send).

Fragment-scanner traces: total exit time ~5.9s → ~3.2s, with `main_exn`
unchanged (~1.8s) — the delta was pure shutdown tail.

Pins the exporter-fix commit `2da6919`; the later CI-pinning commit on
#4 is
workflow-only and doesn't change the built package. Re-point to the
`fork` merge
commit once #4 lands. Needs the `opam` label so lockfile CI regenerates.

Co-authored-by: ajbt200128 <austin@cutedogs.org>
Co-authored-by: Katrina Liu <katrina@semgrep.com>
Co-authored-by: Joshu Coats <joshua.coats@semgrep.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

synced from Pro 08ba4c961d5a9123bfb903efae0e29221f1e1386
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants