-
Notifications
You must be signed in to change notification settings - Fork 594
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(storage): add metrics for iter_log #16658
Conversation
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.
Rest LGTM
); | ||
|
||
let iter_in_progress_counts = register_guarded_int_gauge_vec_with_registry!( |
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.
Prior to this PR, this is protected via RelabeledCounterVec
but this PR removes it. Is this expected?
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.
The corresponding metrics of iter_in_process_counts
in this PR is iter_counts
, which is still protected by RelabeledCounterVec
.
This is actually newly added metrics that monitors existing on going iterators of each table. Not sure if we should expose it in per table granularity to observe the existing number of iterators of each table.
) | ||
.unwrap(); | ||
|
||
let iter_log_op_type_counts = register_guarded_int_counter_vec_with_registry!( |
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.
Do we want to expose this metric in per table granularity by default? If not, we can use RelabeledCounterVec.
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.
Only the tables that call iter_log
will create a new label on this metrics, which is expected to be few, with just mv tables that enable log store. So I think it's fine to expose the metrics in per table granularity so that we can get more insights about the log table.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Add metrics for
iter_log
inMonitoredStateStore
.The code of
MonitoredStateStoreIter
is refactored so that we can reuse most monitoring code ofiter
. Besides the original metric ofiter
, we also add counter to measure the number of insert, update delete initer_log
value.A metrics that monitoring existing concurrent iter is added by the way in this PR.
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.