-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-24786][state] Introduce and expose RocksDB statistics as metrics #19701
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
Conversation
masteryhx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
I've left some comments, PTAL.
docs/layouts/shortcodes/generated/rocksdb_native_metric_configuration.html
Show resolved
Hide resolved
| // RocksDB statistics based metrics, report at database level | ||
| // -------------------------------------------------------------------------------------------- | ||
|
|
||
| public static final ConfigOption<Boolean> MONITOR_BLOCK_CACHE_HIT = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also create a new class called "RocksDBStatistics" and use it just like "RocksDBProperty" ?
Maybe it could be clearer to me. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, I think previous RocksDBProperty is a bit complex, and I prefer to use the full metrics name in the source code as it could be easily searched.
...cksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBNativeMetricOptions.java
Show resolved
Hide resolved
|
LGTM |
|
@flinkbot run azure |
1 similar comment
|
@flinkbot run azure |
|
@masteryhx Thanks for the review, and I will merge this PR since the CI is green finally. |
What is the purpose of the change
Introduce and expose RocksDB statistics as metrics.
Brief change log
RocksDBNativeMetricMonitor.Verifying this change
This change added tests and can be verified as follows:
RocksDBNativeMetricMonitorTest.Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation