Add pipeline processing and completion to time series stream, add pipeline latency graph to Performance tab#6650
Add pipeline processing and completion to time series stream, add pipeline latency graph to Performance tab#6650Karakatiza666 wants to merge 2 commits into
Conversation
[web-console] Add pipeline latency graph to Performance tab Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
|
Is there a reason not to always show p99? The checkbox is awkward. It's worth having a link to https://docs.feldera.com/pipelines/latency/ Also, 3ms + 3ms doesn't make sense. Completion latency includes step latency. |
In the title I subtract the current completion from step latency to show the "internal Feldera" latency and sink latency separately. I can revert to just showing the absolute latency values. Graphs and the tooltips show the absolute values, of course
I only made it optional to avoid overloading the chart by showing 4 lines in the graph by default. I am happy to show p99 unconditionally if that's the consensus |
|
To make the title less confusing I could reword it to "X ms processing + Y ms output" |
Easier so it corresponds to the graph: |
|
Ok, putting completion first, LMK if I should switch them around (again, to highlight Feldera itself is fast) |
|
The other way around is fine too. |
mythical-fred
left a comment
There was a problem hiding this comment.
Nice addition. The backend pieces (histogram quantile(), per-connector aggregation, four new time-series fields with skip_serializing_if) and the frontend (adaptive formatDuration, p99 toggle, tightened baseline y-axis when hidden, oscillation-safe compact title) all read cleanly, and both sides have thorough tests. CI still pending at review time, but the code is signed-off and free of AI-tool trailers. Approving with one design question and two nits.
Design question — "p99" is p99 of medians, not tail latency
ControllerStatus::latency_percentiles collects each connector's median into a vector and takes p50/p99 across that vector (stats.rs lines ~757-793). With the number of input connectors a typical pipeline has (often 1-3), the "p99 across connectors" line is arithmetically identical to the max of medians and does not reflect within-connector tail latency at all. The graph legend labels it p99 and the tooltip renders it prominently, which is likely to be read as "the slowest 1% of records", not "the slowest median among connectors".
Two alternatives worth considering:
- Per-connector
p99first, then aggregate. Pullquantile(0.99)from each connector's histogram (you already added the method) and take the max (or p99) across connectors. That way each series answers a well-defined tail-latency question even with one connector. - Combined-histogram approach. Fold the per-connector histograms into one before quantile-ing. Buckets are exponential and shared, so you can add them element-wise.
Not a merge blocker — the graph still surfaces slow connectors, which is useful — but the current label sets user expectations the metric can't meet. At minimum, the tile title / axis label could be honest about it ("slowest connector median" or similar).
Nits (non-blocking)
ExponentialHistogramSnapshot::quantilereturns the lower bound of the containing bucket (bucket_to_range(index).start()). For high-magnitude latencies the exponential bucket widths get proportionally large, so a value in bucket[900_000, 999_999]reports as900_000— a ~10% systematic underestimate on the tail exactly where accuracy matters most. Cheap fix: return the midpoint or the upper bound; the existing testquantile_known_distributionwould need one line updated. Or at least call out the bias in the rustdoc so a future reader doesn't chase a phantom "why is p99 lower than reality" bug.niceLatencyMax([])returns1000(µs), so an empty latency series draws a 0-1ms axis. Fine for the graph's cold-start moment, but combined with thehasDataboolean you compute and never surface to the UI, a "no latency samples yet" state would be more honest than a fake 1ms axis. Consider rendering a placeholder in the tile when!hasData.
CI is still pending; if any check fails, please re-ping.
|
|
||
| /// Latency percentiles (microseconds) across input connectors, for the | ||
| /// time-series graph. Each connector contributes its own median latency, | ||
| /// we then take p50 and p99 across connectors, for both |
There was a problem hiding this comment.
Design nit: this is "p99 across connectors of their medians", not "p99 of record latencies". With N input connectors typically small (1-3), the p99 line is arithmetically the max of medians and does not reflect the tail distribution any connector actually sees. Consider (a) calling quantile(0.99) on each connector's histogram and aggregating that, or (b) folding the per-connector histograms into one before quantile-ing. See the review body.
| let mut cumulative = 0u64; | ||
| for (index, count) in self.buckets.iter().enumerate() { | ||
| cumulative += *count; | ||
| if cumulative >= rank { |
There was a problem hiding this comment.
Reporting the bucket's lower bound biases the estimate downward, and the bias scales with the value: a 950ms sample lands in [900_000, 999_999] and reports as 900ms — a ~10% underestimate on the p99 tail where accuracy matters most. The bucket midpoint (or upper bound) is a cheaper reporting choice with lower expected error; alternatively, at least document the systematic bias in the rustdoc so a reader isn't confused why p99 always looks fast.
|
…ng the chart Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
|
@Karakatiza666 let me know when this is ready again for review after you addressed the comments. |
|
An aggregate completion latency graph that somehow summarizes across all connectors may be misleading. Different data sources may have very different latency behaviors, e.g., a Kafka connector that ingests many small messages can have latency in milliseconds, while a Delta connector that processes an occasional large merge can take multiple seconds or minutes. I don't know what p99 even means here, since we have two dimensions: N connectors x M seconds reporting window. |
Every pipeline already stores distribution of per-record latencies for every connector, in buckets. This PR calculates a median latency per connector based on these buckets, and then aggregates latency across all connectors as p50 and p99, for input-to-processing and input-to-completion latency. The whole-pipeline step latency metric is not used for calculation.
I chose to show two latencies because I want users to distinguish between Feldera's internal latency and completion latency, which is impacted by the output sink performance
Testing: manual, added unit tests