mirror of
https://github.com/livekit/livekit.git
synced 2026-10-24 04:13:11 +00:00
dc/fec_buffer_refactor
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
6ed445bad1 |
sfu: report forwarding latency as p90 instead of the mean (#4920)
* sfu: report forwarding latency as p90 instead of the mean The forwarding-latency metric was the mean transit over all forwarded packets. A mean is dominated by a few slow outliers, so a handful of packets stalled on the forward path (e.g. goroutine scheduling latency) inflated the whole node's reported latency even when nearly every packet was forwarded promptly. Report p90 instead: p90 rising means roughly a tenth of forwarded packets are slow, a broad signal of systemic forwarding load rather than a sparse tail. To read a percentile over the report window, the mergeable per-interval summary now keeps a small power-of-two-bucket histogram of transit instead of running moments (sum, sum-of-squares). Drop the jitter (transit std dev) gauge: nothing consumed it, and any spread is derivable from the forward-latency histogram. The protobuf ForwardJitter field is left in place, now unset, to deprecate separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sfu: clamp forwarding percentile to the observed [min, max] Bucket interpolation assumes a uniform fill, so a single 20ms packet (or uniform traffic) could report a p90 above every observed sample. Clamp the interpolated value to the summary's already-tracked min/max, so a quantile never falls outside the data. Exact for single samples and repeated identical latencies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sfu: fix stale p50 comment in the percentile test The reported metric is p90; the test comment still said p50 replaced the mean. Reword it to reflect that a percentile, not the mean, is reported. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sfu: quarter-octave forwarding-latency buckets for threshold resolution Octave buckets are too coarse near the overload thresholds: 300us falls in [256,512), so a p90 clustered at ~265us and one at ~500us interpolate to the same value and would trip (or not) identically. Split each octave into four linear sub-buckets so the two land in different buckets, on the correct side of the threshold. Min/max clamping alone does not fix this once a node has a high tail, since its max no longer bounds the interpolation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sfu: interpolate percentiles within each bucket's observed range Replace the quarter-octave split with plain octave buckets that also carry the observed [min, max] of their samples, and interpolate a percentile within that range instead of the bucket's nominal edges. This is exact when a bucket's samples cluster, so a p90 near an overload threshold that falls mid-bucket lands on the correct side of it regardless of bucket width -- no threshold-aware boundaries needed. It subsumes the min/max clamp, since an estimate can no longer leave the observed samples. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sfu: cover the 500us cluster in the threshold test Assert milos's full review example exactly: 265us and 500us clusters that octave-nominal interpolation both read as ~341us now read 265us and 500us. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sfu: make forwardSummary.addSample a pointer receiver The summary grew from a small moments struct into a per-bucket histogram (~700 bytes), so the value-receiver addSample copied the whole summary on every drained sample in the flush loop. Mutate in place instead: ~28ns -> ~2ns per sample in the background fold, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sfu: eighth-octave buckets for accuracy near thresholds Split each octave into 8 linear sub-buckets (was plain octaves). With the per-bucket min/max interpolation this reports a tight p90 cluster exactly even when it sits mid-octave: 850x265us + 150x410us now reads 410us (above a 400us threshold) instead of 395.5us, and a lognormal p90 lands within ~0.2us of exact. Per-sample add cost is unchanged (~2ns); cost is ~5KB per summary and a larger but per-report merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
2bd3b9d67f |
Add a method to get forward stats via API method. (#4664)
Use by cloud simulated tracks. Introduces a lock, but it is used only in the background worker (where it will be almost always uncontested) and when API GetStats is accessed. Does not touch the per-packet Update path. |
||
|
|
345bc5eeb0 |
Move ForwardStats aggregation off the packet forwarding path (#4660)
* Move ForwardStats aggregation off the packet forwarding path ForwardStats is a process-wide singleton and its Update runs for every forwarded packet. It previously took a shared mutex, updated a windowed aggregate, and observed into a global Prometheus histogram on every call. Update now only buffers the transit sample into a sharded lock-free ring (one atomic add to reserve a slot, one atomic store to publish). A background worker drains the ring every summary interval, observes each sample into the histogram (per-packet fidelity retained) and folds the interval summary into a window ring for the latency/jitter gauges. Under sustained overload the oldest excess samples are dropped and counted, and the count is logged. Ring slots are atomic.Int64 so producer store / consumer load are synchronized (race-clean). Capacity is numShards*shardCap = 131072 samples; at the default 50ms summary interval that sustains ~2.6M samples/s before dropping. Benchmark: BenchmarkForwardStatsUpdate (0 allocs/op both), Update per call: cores before after speedup 1 ~8.6 ns ~1.5 ns ~6x 8 ~175 ns ~22 ns ~8x The before path (mutex + windowed Welford aggregate + per-packet histogram observe) degrades ~20x from 1 to 8 cores under contention; the after path does not block and stays an order of magnitude lower under load. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix ring publish race and report-path double-flush Addresses two review findings on the forward-stats sample buffer. 1. Publish race (reserve-before-store): push reserved a ring slot by advancing writeIdx and stored the value afterwards, so drain could read a slot the producer had reserved but not yet written, getting a stale/zero value; the producer's later store then landed behind the read cursor and was lost. Each slot now carries a publish epoch. push stores the value and then publishes seq = index+1. drain reads a slot only when seq == r+1; a reserved-but-unpublished slot at the cursor stops the drain and is picked up on the next call, so no sample is read stale or lost. A slot overwritten during the read is detected on re-check and counted as dropped. The windowed latency/jitter stats did not permanently drift even before this change (report recomputes from a fixed-size ring that overwrites, not discounts, old buckets, so any error aged out within the report window), but these values feed the capacity manager, so the buffer is made exact. 2. Report-path double-flush: run() flushed on both the summary tick and the report tick, and every flush advances the window ring, so the ring cycled faster than intended and the effective window was shorter than configured (~5% at 50ms/1s/1m). The report tick no longer flushes; the summary ticker keeps the ring current to within one summary interval. Benchmark, Update per call (0 allocs/op), 1 and 8 cores: after fixes: ~5.5 ns / ~29 ns before fixes (this branch): ~1.5 ns / ~22 ns baseline (mutex + per-packet histogram): ~8.6 ns / ~175 ns The publish epoch adds one atomic store to push; the path stays non-blocking, zero-allocation, and well below the baseline under contention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |