Skip to content

feat: decouple storage sinks from the USB drain; surface host backpressure - #96

Closed
boringethan wants to merge 2 commits into
nextfrom
feature/histo-stream-decouple-backpressure
Closed

boringethan wants to merge 2 commits into
nextfrom
feature/histo-stream-decouple-backpressure

Conversation

@boringethan

@boringethan boringethan commented Jun 22, 2026 •

Copy link
Copy Markdown
Contributor

Refs #116. Superseded by #113 (same work rebased + review fixes); kept open as fallback until #113 merges, then this closes.

Why

On the 2026-06 long-scan soak the sensor firmware logged ~833 HISTO enqueue fail: queue full events (enq-deq=4, txfail=0). Root cause: the ScanRunner consume loop runs on the same thread that drains the live USB histogram batch queue, so a slow disk/DB sink — a SQLite commit/fsync or a CSV flush — stalls that drain. The stall backpressures the parser → dev.read → the firmware's deliberately shallow 4-deep histo queue (~100 ms at 40 Hz) overflows and frames are dropped. These were brief (>100 ms, <1 s) host-side stalls, not a USB fault and not the persistent histo-wedge.

What

Fix #1 — decouple the storage sinks from the USB drain

  • New AsyncSink (omotion/pipeline/async_sink.py) wraps a sink and runs consume() on a dedicated worker thread fed by a bounded FIFO buffer, so a transient sink stall is absorbed by the buffer instead of stalling the USB path. on_scan_start stays synchronous (preserves critical-sink abort + ordering before the first consume); on_complete drains the buffer and joins the worker before finalising the wrapped sink, so no persisted data is lost. Sustained overload still backpressures once the buffer fills (correct — you can't outrun the disk forever).
  • ScanWorkflow now wraps CsvSink and ScanDBSink in AsyncSink (the live-plot/diagnostics sinks stay synchronous so the GUI trace isn't delayed).
  • LiveUsbSource._batch_queue depth is now configurable and defaulted up from 4 (~1 s) to 16, so other runner hiccups (pipeline CPU spike, GC) are absorbed before reaching dev.read.

Fix #3 — make the previously-invisible sub-second backpressure observable

  • StreamInterface now times every packet-queue put. A put that blocks ≥100 ms (previously only the >1 s drop was logged) is counted per scan — backpressure_stalls / backpressure_stall_s / dropped_chunks — rate-limit warned mid-scan, and summarised in the stop_streaming line (logged at WARNING if any host-side drops occurred).

Out of scope: bumping the firmware HISTO_QUEUE_SIZE (4→8) — RAM-bound on the sensor, intentionally left alone. This PR removes the host-side cause instead.

Tests

New, test-first:

  • test_async_sink.py — FIFO ordering, drain-before-finalise, critical-abort propagation, worker error isolation, Sink protocol conformance, transparency through a real ScanRunner, wrapped accessor.
  • test_stream_interface_backpressure.py — no-stall fast path, sub-second stall counting, drop-past-timeout, counter reset on start_streaming.
  • test_sources.py — configurable / deepened _batch_queue.

Full pipeline + scan-workflow suites pass. (The 3 pre-existing ScanDBSink "empty session deleted" failures on next are unrelated to this change and untouched here.)

🤖 Generated with Claude Code

boringethan and others added 2 commits June 22, 2026 15:26
…ssure

The ScanRunner consume loop runs on the same thread that drains the live
USB histogram batch queue, so a slow disk/DB sink (SQLite commit/fsync, CSV
flush) stalls that drain, backpressures the parser and dev.read, and
overflows the sensor firmware's 4-deep histo queue — dropping histogram
frames. Root-caused on the 2026-06 long-scan soak ("HISTO enqueue fail:
queue full ... enq-deq=4 txfail=0": brief host-side drain stalls, not a USB
fault and not the persistent histo-wedge).

Fix #1 — decouple storage sinks from the drain:
- New AsyncSink wraps a sink and runs consume() on a dedicated worker
  thread fed by a bounded FIFO buffer, so a transient sink stall is
  absorbed instead of stalling the USB path. on_scan_start stays
  synchronous (preserves critical-sink abort + ordering); on_complete
  drains the buffer and joins before finalising, so no persisted data is
  lost. ScanWorkflow now wraps CsvSink and ScanDBSink.
- LiveUsbSource._batch_queue depth is configurable and defaulted up from 4
  (~1 s) to 16 so other runner hiccups (pipeline CPU spike, GC) are
  absorbed before reaching dev.read.

Fix #3 — make sub-second host backpressure visible:
- StreamInterface times every packet-queue put. A put that blocks >=100 ms
  (previously only the >1 s drop was logged) is counted per scan
  (backpressure_stalls / backpressure_stall_s / dropped_chunks),
  rate-limit warned, and summarised at stop_streaming.

(Firmware queue-depth bump is intentionally out of scope — RAM-bound.)

Tests: AsyncSink (FIFO ordering, drain-before-finalise, critical-abort
propagation, worker error isolation, Sink protocol, runner transparency,
wrapped accessor), configurable batch-queue depth, StreamInterface
backpressure counters.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the magic batch-queue default (16) with a value derived from a
3 s buffer target and the capture rate (CAPTURE_HZ) + batch size, so the
intent is self-documenting and survives a different batch_size_frames. At
the default 10-frame batch this is 12 batches (~3 s); the 64-deep packet
queue adds ~1.6 s behind it. batch_queue_size stays overridable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@boringethan

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #113, which carries both of this PR's commits rebased onto current next plus fixes for the review findings (batch-queue sizing, AsyncSink finalize race, reuse poisoning, warning suppression). Tracking continues on #116.

@boringethan boringethan closed this Jul 2, 2026
boringethan added a commit that referenced this pull request Sep 29, 2026
…covery

Brings the merged OB-telemetry work (PR #170 / sensor-fw #104, wire version 2)
into the bench branch so the parser matches the flashed firmware
1.8.2-dev.1-11-g93f3ca4 — the version/size guard rejects every v1 snapshot
otherwise. Adds z_avg (the four zero-line dark-row averages, /64 -> DN),
z_avg_mean/spread, blk_lvl_target, blc_thres, the black/zero-line windows, and
the BLC fault latches to get_camera_telemetry() and CameraTelemetryCsvLogger.
Also pulls the HISTO-pipe recovery (PR #172 / sensor-fw #106) so a telemetry
poll that stalls a reader no longer wedges the histogram stream (issue #96).

Files taken verbatim from origin/next (my branch never diverged them). Bench
validation on the RIGHT sensor: z_avg_mean = 126.6 DN pinned to the 128 servo
target, spread 0.5 DN, fault latch clean; histograms + telemetry both survived
a 60 s co-polled scan. 15/15 telemetry unit tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
boringethan added a commit that referenced this pull request Sep 29, 2026
The first warm-restart run (WRESTART) failed after the cold reference scan:
every powered-idle handoff timed out on connect (Errno 10060). Root cause is the
sensor-fw #96 COMM stall -- sustained 1 Hz camera-telemetry polling wedges the
command endpoint ~9-18 min into a scan. A rig-OFF cooldown hides it because the
next scan's power cycle recovers the endpoint; a powered-idle gap keeps the rig
ON, so the stall carries into the next connect and it never recovers. The raw
histogram stream (die temp + intensity) is unaffected and is all the warm-restart
ladder needs, so telemetry is now auto-disabled with a warning whenever any
ladder step is powered-idle.

Also: powered-idle now follows the step's fan_on flag instead of forcing the fan
off, so the gap can cool the module from the streaming plateau at a chosen rate
(fan-on gives an informative spread of restart dip depths; fan-off barely cools).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
boringethan added a commit that referenced this pull request Oct 1, 2026
1 Hz camera-telemetry polling alongside the image stream stalled the sensor
COMM endpoint ~3 min into FFDIP_01 (USBTimeoutError 10060, cf. sensor-fw #96),
which also blinds thermal_soak's --max-die-c stop. Use the soak's 10 s default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant