Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update iceberg writer and add more metrics #13893

Merged
merged 2 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ criterion = { version = "0.5", features = ["async_futures"] }
tonic = { package = "madsim-tonic", version = "0.4.1" }
tonic-build = { package = "madsim-tonic-build", version = "0.4.2" }
prost = { version = "0.12" }
icelake = { git = "https://github.com/icelake-io/icelake", rev = "5cdcdffd24f4624a0a43f92c5f368988169a799b", features = [
icelake = { git = "https://github.com/icelake-io/icelake", rev = "3f7b53ba5b563524212c25810345d1314678e7fc", features = [
"prometheus",
] }
arrow-array = "49"
Expand Down
2 changes: 1 addition & 1 deletion docker/dashboards/risingwave-dev-dashboard.json

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions e2e_test/iceberg/test_case/cdc/load.slt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ CREATE SINK s1 AS select * from products WITH (
primary_key = 'id'
);

statement ok
flush;

query I
select count(*) from products;
----
Expand Down
42 changes: 36 additions & 6 deletions grafana/risingwave-dev-dashboard.dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -3398,32 +3398,62 @@ def section_iceberg_metrics(outer_panels):
"Iceberg Sink Metrics",
[
panels.timeseries_count(
"Write Qps Of Iceberg File Appender",
"iceberg file appender write qps",
"Write Qps Of Iceberg Writer",
"iceberg write qps",
[
panels.target(
f"{metric('iceberg_file_appender_write_qps')}",
f"{metric('iceberg_write_qps')}",
"{{executor_id}} @ {{sink_id}}",
),
]
),
panels.timeseries_latency(
"Write latency Of Iceberg File Appender",
"Write Latency Of Iceberg Writer",
"",
[
*quantile(
lambda quantile, legend: panels.target(
f"histogram_quantile({quantile}, sum(rate({metric('iceberg_file_appender_write_latency_bucket')}[$__rate_interval])) by (le, sink_id))",
f"histogram_quantile({quantile}, sum(rate({metric('iceberg_write_latency_bucket')}[$__rate_interval])) by (le, sink_id))",
f"p{legend}" + " @ {{sink_id}}",
),
[50, 99, "max"],
),
panels.target(
f"sum by(le, sink_id)(rate({metric('iceberg_file_appender_write_latency_sum')}[$__rate_interval])) / sum by(le, type, job, instance) (rate({metric('iceberg_file_appender_write_latency_count')}[$__rate_interval]))",
f"sum by(le, sink_id)(rate({metric('iceberg_write_latency_sum')}[$__rate_interval])) / sum by(le, type, job, instance) (rate({metric('iceberg_write_latency_count')}[$__rate_interval]))",
"avg @ {{sink_id}}",
),
],
),
panels.timeseries_count(
"Iceberg rolling unfushed data file",
"",
[
panels.target(
f"{metric('iceberg_rolling_unfushed_data_file')}",
"{{executor_id}} @ {{sink_id}}",
),
]
),
panels.timeseries_count(
"Iceberg position delete cache num",
"",
[
panels.target(
f"{metric('iceberg_position_delete_cache_num')}",
"{{executor_id}} @ {{sink_id}}",
),
]
),
panels.timeseries_count(
"Iceberg partition num",
"",
[
panels.target(
f"{metric('iceberg_partition_num')}",
"{{executor_id}} @ {{sink_id}}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the exec id in RW? I suggest using actor id instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the exec id in RW? I suggest using actor id instead

I think it means

pub identity: String,
, because the sink metrics is per executor, executor id is used to identify it.🤔

Why not use, cc @wenym1

pub executor_id: u64,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be compatible with the code before I introduce this refactor, because before I introduce this refactor, the sink metric was using the executor id to form a identity string as the label.

I think we can change to directly using the actor id or executor id, but we should remember to change the grafana dashboard at the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's refactor them all in a separate PR.

),
]
),
]
)
]
Expand Down
2 changes: 1 addition & 1 deletion grafana/risingwave-dev-dashboard.json

Large diffs are not rendered by default.

Loading
Loading