fix(provider): stop OVT pull re-DESCRIBE storm from a reachable-but-silent origin

[naanlizard]

The below is Claude-written (as is the change), from analysing crash dumps we've been seeing in prod, plus a back-and-forth with @getroot on the approach. The crash itself (the OvtStream::_description race) is already fixed on master by #2165; this fixes the OVT connection storm that was driving it.

Reworked to @getroot's suggested approach: instead of a re-pull backoff with new PullStream state, reset the no-input clock on a successful reconnect.

Problem

When an edge relays a stream from an origin and the source stops sending media while its TCP connection stays up (a stalled encoder whose host keeps answering at the socket level), the origin keeps the stream alive. The edge's WhiteElephantStreamCollector then enters a tight re-pull loop:

• It runs every 100 ms. A pull stream with no incoming packets for NoInputFailoverTimeout is Stop()-ed; on the next tick it is STOPPED, so the collector re-pulls it — reconnecting and re-DESCRIBEing the origin, with no delay.
• The reconnect succeeds (the stream still exists at the origin) but delivers no media, so last_recv stays old and the stream is stopped and re-pulled again on the very next tick.

Three edges relaying one silent stream produce a continuous DESCRIBE storm (~25/s per stream) at the origin's OVT publisher.

(The collector re-pull loop — and thus this storm — only engages when RetryCount > 0; with the default RetryCount = 0, ResumeInternal terminates a silent pull stream on its first re-pull instead of reconnecting. The reproduction and our deployment use RetryCount = 2.)

Reproduction

Minimal Docker reproduction (origin + 3 edges + a publisher frozen with docker pause to model a silent-but-connected source): https://github.com/naanlizard/ome-ovt-flap-repro

Fix

Measure the no-input timeout from max(last_recv, last_reconnect) instead of last_recv alone:

• The collector keeps a thread-local map of the last time it resumed each pull stream, stamped only on a successful reconnect.
• A successful reconnect to a reachable-but-silent origin delivers no media, so last_recv stays old; measuring from max(last_recv, last_reconnect) means the stream is not stopped again until a full NoInputFailoverTimeout has elapsed since the reconnect. The re-pull (re-DESCRIBE) cadence therefore settles at NoInputFailoverTimeout (default 3 s) instead of once per 100 ms tick.
• No new PullStream members and no lock — the map is touched only by the collector thread and pruned each tick for streams that no longer exist. The monitoring last_recv is never written, so reported stats stay honest. Both collector resume sites (the no-input retry and the failback URL switch) reset the clock.

Logging

The silent-origin lifecycle is now visible in the edge log:

• reconnect — debug: each time the collector re-pulls and reconnects to the origin. Off at the default level; raise the edge to debug to watch the cadence.
• no input → stop — warning (unchanged): prints the true time since the last media packet, which keeps growing while the origin stays silent.
• media resumed — info: once, when media starts flowing again after a re-pull.

Behaviour (from the reproduction, 3 edges, one silent stream, 20 s window)

	before	after
origin OVT re-pulls	506 (~25/s)	36 (≈ one per NoInputFailoverTimeout per edge)
edge no incoming packets stops	255	18
failover to a healthy backup URL	immediate	immediate
recovery when the source returns	immediate (via viewer pull)	immediate (via viewer pull)

NoInputFailoverTimeout / RetryCount / UnusedStreamDeletionTimeout semantics are unchanged.

Behaviours to be aware of

• While retries are enabled, a reachable-but-silent origin is re-DESCRIBEd once per NoInputFailoverTimeout indefinitely; it does not reach TERMINATED. A successful reconnect resets _restart_count, so the relay is never abandoned while the origin is reachable — by design. This change caps the DESCRIBE rate at the timeout cadence; it does not end the relay. Tearing such a stream down is what PacketSilenceTimeoutMs on the origin is for — the origin-side fix is the root cause; this patches the storm in the window before the origin removes the silent stream.
• The stop log reports true media-silence, not the value that triggered the stop. The decision uses max(last_recv, last_reconnect), but the log prints the real time since the last media packet (so it can exceed NoInputFailoverTimeout after a silent reconnect). This keeps the logged number honest.
• The clock resets on real media, not on signaling. last_recv advances only on media packets, not on the OVT DESCRIBE/PLAY exchange, so a silent reconnect never resets it.

[naanlizard]

An aside, since this is a convenient way to ping y'all - @getroot @Keukhan

I have been digging into color space (601/709, full/limited) and screenshots and how OME handles them. I'm in the final review phase of a PR to

1. Fix color rendering on thumbnails generated by OME
2. Fix a bug with webP thumbnail encoding performance
3. Add AVIF image output format support
4. Add AV1 ingest thumbnail support

I'll hopefully push that in a few hours or tomorrow for review, including a big page I've been using to compare screenshots generated by various versions of OME (master, 0.20.5, my fork, and the ffmpeg upgrade commit that is part of my work)

[getroot]

@naanlizard A very large refactoring is going to land on the Transcoder side this week. It would be better to put up any PRs that touch the transcoder after that.

[naanlizard]

Is there somewhere I can see that work in progress?

[getroot]

Nice analysis, and the backoff works. Personally I'd prefer to fix this with a more minimal change, since you already noted last_recv only advances on real media.

Instead of adding the backoff state, I'd just reset the no-input clock on a successful reconnect. The re-pull cadence then settles at NoInputFailoverTimeout on its own, with no new members and no lock, and existing viewers recover within the timeout instead of up to the 30s cap.

For the implementation, my idea is to keep a collector-local "last reconnect" timestamp and measure no-input from max(last_recv, last_reconnect) rather than stamping the monitoring last_recv itself, so the stats stay honest.

What do you think?

[getroot]

Is there somewhere I can see that work in progress?
@naanlizard It is the PR.
#2202

[naanlizard]

I'd defer to your judgement, of course. That would be 3s (NoInputFailoverTimeout) between DESCRIBEs instead of the scaling-to-30s, right?

The backoff seems better to me because 3s by default is still a bit tight, and if someone had a ton of edges, it could still cause a bunch of DESCRIBEs, but I guess that's an edge case. Your solution is certainly cleaner.

An aside, PacketSilenceTimeoutMs defaults to 0 for RTMP, should that perhaps default to something like 5-10s? I'm not sure there are many situations where you have no media packets coming in for e.g. 5s and you should stay connected

[getroot]

The root cause here is that a stream stays alive even when there is no data flow.
Setting PacketSilenceTimeoutMs on the origin resolves this, and the request storm from the edge goes away.
On the edge side, I think it would be good to apply the bug fix I described. It patches the storm during the window before the stream is removed by PacketSilenceTimeoutMs on the origin.
With that, I don't see a case that still needs the backoff. Other than this situation, are there cases where you think the backoff would still be needed?
And if some case intentionally needs to keep a silent stream alive, setting NoInputFailoverTimeout to a very large value covers it.

As for changing the default value of PacketSilenceTimeoutMs, let's handle that separately in a Discussion. There was a reason I set it to 0, but I don't remember it well right now, so I'll think it over.

[naanlizard]

Updated

[naanlizard]

Rebased onto current master (~ v0.21.0); the branch had gone stale against the src/projects flatten (#2248). The fix itself is unchanged — still the approach suggested above: no-input measured from max(last_recv, last_reconnect) via a collector-local map.

Current master doesn't crash any more, which is good, but there's still a ton of noise from the flapping.