feat(sink): add sink throughput( mb/s) metrics #19587
Conversation
| @@ -200,6 +200,7 @@ impl<F: LogStoreFactory> SinkExecutor<F> { | |||
| let input = input.inspect_ok(move |msg| { | |||
| if let Message::Chunk(c) = msg { | |||
| metrics.sink_input_row_count.inc_by(c.capacity() as u64); | |||
| metrics.sink_input_size.inc_by(c.estimated_size() as u64); | |||
There was a problem hiding this comment.
I think the metrics here will be inaccurate when sink decoupling is on because the metrics here is measuring the throughput of the input channel. When sink decoupling is on, the input will be polled and written to log store regardless of the external sink.
We need to add a metric for log store reader throughput:
There was a problem hiding this comment.
It looks like the metric here is log_store_read_rows, which doesn't match the sink_input_row_count we defined earlier. Should we merge the two or add another log_store_read_rows（mb/s）
| panels.timeseries_bytesps( | ||
| "Sink Throughput(MB/s)", | ||
| "The figure shows the number of bytes written each sink per second. For sinks with 'sink_decouple = true', please refer to the 'Sink Metrics' section", | ||
| [ | ||
| panels.target( | ||
| f"(sum(rate({metric('stream_sink_input_size')}[$__rate_interval])) by (sink_id) * on(sink_id) group_left(sink_name) group({metric('sink_info')}) by (sink_id, sink_name)) / (1000*1000)", | ||
| "sink {{sink_id}} {{sink_name}}", | ||
| ), | ||
| ], | ||
| ), | ||
| panels.timeseries_bytesps( | ||
| "Sink Throughput(MB/s) per Partition *", | ||
| "The number of bytes streamed into each sink per second. For sinks with 'sink_decouple = true', please refer to the 'Sink Metrics' section", | ||
| [ | ||
| panels.target( | ||
| f"(sum(rate({metric('stream_sink_input_size')}[$__rate_interval])) by (sink_id, actor_id) * on(actor_id) group_left(sink_name) {metric('sink_info')}) / (1000*1000)", | ||
| "sink {{sink_id}} {{sink_name}} - actor {{actor_id}}", | ||
| ), | ||
| ], | ||
| ), |
There was a problem hiding this comment.
What's the meaning of * at the end of panel title? And why it's missing in "Sink Throughput(MB/s)"?
| # - use: prometheus # metrics | ||
| # - use: tempo # tracing | ||
| # - use: grafana # visualization | ||
| - use: prometheus # metrics | ||
| - use: tempo # tracing | ||
| - use: grafana # visualization |
| @@ -73,6 +73,7 @@ pub struct StreamingMetrics { | |||
|
|
|||
| // Sink | |||
| sink_input_row_count: LabelGuardedIntCounterVec<3>, | |||
| sink_input_size: LabelGuardedIntCounterVec<3>, | |||
There was a problem hiding this comment.
What about sink_input_bytes which is more consistent with the source one?
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Fix #19290.
Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.