Skip to content

byoc: drain data subscriber on /stream/stop before tearing down#3927

Closed
rickstaa wants to merge 1 commit into
trickle-next-endpointfrom
fix/byoc-data-subscriber-drain
Closed

byoc: drain data subscriber on /stream/stop before tearing down#3927
rickstaa wants to merge 1 commit into
trickle-next-endpointfrom
fix/byoc-data-subscriber-drain

Conversation

@rickstaa

@rickstaa rickstaa commented May 8, 2026

Copy link
Copy Markdown
Member

Draft. Pending end-to-end validation against examples/runner/live_transcribe once CI publishes a docker image for this branch.

Fixes #3924.

The bug

The gateway's data SSE proxy was tearing down its trickle subscriber as
soon as /stream/stop arrived, which races the runner's on_stream_stop
hook. Any final emit_data the runner publishes during its stop hook
(e.g. live_transcribe's last partial-window flush) is written to the
trickle channel after the gateway has already cancelled its
subscriber's context — so the segment is never read, never relayed to
the SSE caller.

Reproduction (3/3)

Against livepeer-python-gateway/examples/runner/live_transcribe, running the existing ./test.sh 3 times:

Run Runner emitted Gateway delivered Dropped
1 7 (transcript 0–6) 6 (transcript 0–5) transcript[6] (on_stream_stop flush)
2 7 6 transcript[6]
3 7 6 transcript[6]

Gateway log on each run:

trickle subscribe error reading: failed to complete GET for next segment:
Get .../{stream}-data/N: context canceled

The data subscriber was mid-GET on the next segment when ctx cancellation fired.

The fix

Decouple the data subscriber's context from /stream/stop. After the
stream context is cancelled, give the subscriber up to drainTimeout
(10s) to keep reading. The trickle channel closes cleanly when the
runner SDK tears down its data publisher (DELETE → next Read returns
trickle.EOS), at which point the loop exits via the existing EOS
path. The timeout caps the drain so a hung runner can't keep the
goroutine alive forever.

What changed

byoc/trickle.go:startDataSubscribe:

  • Introduced drainCtx rooted in context.Background() so it doesn't get cancelled by /stream/stop
  • Watchdog goroutine waits for ctx.Done() (stream stop), then starts a 10s timer before cancelling drainCtx
  • Subscriber + loop's exit check now use drainCtx instead of ctx
  • inputStreamExists() guard is skipped once ctx is already cancelled — during drain the stream tracking is gone but we still want to read pending segments
  • defer drainCancel() ensures the watchdog goroutine never leaks

Companion SDK fix

Tracked at livepeer/livepeer-python-gateway#12 (probe + fallback fix landed in PR #3925 + #3926). This PR closes the data-channel half of the live_transcribe drop story.

Test plan

  • go build ./... passes
  • go test ./byoc/... passes (existing tests; no new ones added in this PR)
  • CI Docker image build green
  • Manual E2E: rebuild live_transcribe compose against this image, run ./test.sh 3× and confirm 7/7 transcripts deliver
  • Confirm normal-stop (no on_stream_stop emit) doesn't add latency to /stream/stop response

Risk / scope

Worst case adds up to 10s to gateway cleanup when the runner doesn't close its data publisher. Doesn't affect the /stream/stop HTTP response time — the drain happens in a background goroutine. No changes to other subscribers (media-out, events) or to the publisher path.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the go Pull requests that update Go code label May 8, 2026
Fixes #3924. The gateway's data SSE proxy was tearing down its trickle
subscriber as soon as /stream/stop arrived, which races the runner's
on_stream_stop hook. Any final emit_data the runner publishes during
its stop hook (e.g. live_transcribe's last partial-window flush) is
written to the trickle channel after the gateway has already cancelled
its subscriber's context — so the segment is never read, never relayed
to the SSE caller.

Decouple the data subscriber's context from /stream/stop. After the
stream context is cancelled, give the subscriber up to drainTimeout
(10s) to keep reading. The trickle channel closes cleanly when the
runner SDK tears down its data publisher (DELETE → next Read returns
trickle.EOS), at which point the loop exits via the existing EOS path.
The timeout caps the drain so a hung runner can't keep the goroutine
alive forever.

Bug reproduced 3/3 against examples/runner/live_transcribe in the
livepeer-python-gateway SDK: the on_stream_stop final-flush transcript
consistently drops. Fix not yet validated end-to-end pending a test
image build.

Refs livepeer/livepeer-python-gateway#12 (companion SDK-side fix).
@rickstaa rickstaa force-pushed the fix/byoc-data-subscriber-drain branch from 5adcbbb to 5139d15 Compare May 8, 2026 13:14
@rickstaa rickstaa changed the base branch from master to trickle-next-endpoint May 8, 2026 13:14
@codecov

codecov Bot commented May 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.85022%. Comparing base (5d82a42) to head (5139d15).

Files with missing lines Patch % Lines
byoc/trickle.go 0.00000% 13 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                       Coverage Diff                       @@
##           trickle-next-endpoint       #3927         +/-   ##
===============================================================
- Coverage               32.88179%   32.85022%   -0.03157%     
===============================================================
  Files                        171         171                 
  Lines                      42078       42088         +10     
===============================================================
- Hits                       13836       13826         -10     
- Misses                     27199       27219         +20     
  Partials                    1043        1043                 
Files with missing lines Coverage Δ
byoc/trickle.go 12.52033% <0.00000%> (-1.69455%) ⬇️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d82a42...5139d15. Read the comment docs.

Files with missing lines Coverage Δ
byoc/trickle.go 12.52033% <0.00000%> (-1.69455%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rickstaa

rickstaa commented May 8, 2026

Copy link
Copy Markdown
Member Author

Closing — going with the structural fix instead of the gateway patch.

The data SSE proxy race fixed here only affects the legacy curl-through-gateway path. The canonical Python client SDK (livepeer/livepeer-python-gateway#6) subscribes to the trickle channel directly and provides its own output_grace_seconds drain budget, which receives the final on_stream_stop emit cleanly without any gateway-side change.

Once tests + downstream consumers migrate to the SDK path, this gateway path is deprecated. Adding 26 lines of drain logic here doesn't pay off if the path is going away.

Branch left in place at fix/byoc-data-subscriber-drain in case anyone wants to revive — but no plan to land.

@rickstaa rickstaa closed this May 8, 2026
@github-project-automation github-project-automation Bot moved this from Triage to Done in Engineering Roadmap May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

BYOC gateway data subscriber drops final emit_data on /stream/stop (data loss)

1 participant