[coordinator] Add rebalance progress and outcome metrics - #4373
heyallencao wants to merge 4 commits into
Conversation
Expose leader-scoped rebalance gauges and round counters using existing task state. Avoid inferring historical bucket outcomes during recovery, and add regression tests and metric documentation. Refs apache#4055
morazow
left a comment
There was a problem hiding this comment.
Thanks @heyallencao 🚀
Added minor nits
Thanks for the review, @morazow! All three comments are addressed. The 18 targeted rebalance tests and style checks pass. |
|
Perfect! Another suggestion: Could we give the rebalance metrics the same lifetime as the coordinator process? We could introduce a Then each This would avoid resetting counters and unregistering/re-registering metrics on leadership changes, and remove the need for a separate RebalanceMetricGroup:
Rough structure as below: public class RebalanceMetrics {
private final Counter rebalancesCompleted = new ThreadSafeSimpleCounter();
// ...
/** The manager of the current leader term, or null while this server is not the leader. */
private volatile @Nullable RebalanceManager current;
public RebalanceMetrics(MetricGroup metricGroup) {
metricGroup.counter(MetricNames.REBALANCES_COMPLETED_TOTAL, rebalancesCompleted);
// ...
metricGroup.gauge(
MetricNames.REBALANCE_BUCKETS_PENDING,
() -> readCurrent(RebalanceManager::pendingBucketCount));
// ...
}
private long readCurrent(ToLongFunction<RebalanceManager> read) {
RebalanceManager manager = current;
return manager == null ? 0L : read.applyAsLong(manager);
}
void bind(RebalanceManager manager) {
current = manager;
}
void unbind(RebalanceManager manager) {
// a late close from an ended term must not detach the manager that replaced it
if (current == manager) {
current = null;
}
}
}RebalanceManager then takes What do you think about this? The initial ticket wanted per leader term, but that would be misleading: counters are not monotonic and readers could not tell a leader reset from a real drop. |
Own RebalanceMetrics in CoordinatorMetricGroup and register metrics once for the coordinator process lifetime. Preserve counters across leader terms and return zero from gauges on standby. Bind managers on startup and unbind on close. Update lifecycle and recovery tests and document the process-scoped semantics.
morazow
left a comment
There was a problem hiding this comment.
Thanks @heyallencao 🚀
I have added couple improvements 🤝 Then should be good to go!
|
Thanks @morazow! I have addressed all six suggestions:
I also did another self-review. All 19 |
|
Thanks @heyallencao! Looks good 🚀 |
Purpose
Closes #4055.
Expose coordinator-level metrics for monitoring rebalance progress, duration, and outcomes without polling the Admin API.
Brief change log
RebalanceMetrics, owned byCoordinatorMetricGroup. Retain the existing coordinator scope without additional table or bucket labels.RebalanceManagerinstartup()and unbind it on close. Counters survive leadership changes, while gauges return zero on standby.Outcome semantics
A round with any failed or timed-out buckets increments the failure counter. The existing Admin API's
COMPLETEDstatus means that execution has finished, not necessarily that every bucket succeeded; this PR does not change that API behavior.An empty plan counts as a successful round. The cancellation counter increments only when a running rebalance is canceled; repeated cancellation does not increment it again.
Leadership changes and recovery
Counters accumulate across all leader terms served by the same coordinator process and reset only when that process restarts. They remain unchanged on standby and are not transferred between coordinator processes.
Bucket-outcome gauges include only results observed in the current leader term. They remain available after completion or cancellation within that term and are cleared when the next round is registered. All gauges return zero on standby. Since per-bucket outcomes are not persisted, recovering an already finished round leaves the bucket-outcome gauges at zero and does not increment round counters. This avoids reporting historical failures as successes. These zeros mean that no outcomes were observed in the current term, not that the historical round had no failures.
Tests
RebalanceManagerTesttests and bothRebalanceManagerITCaseintegration tests, plus coordinator lifecycle/election and metric-group tests.git diff --checkpassed.API and Format
No changes to public client APIs, RPC messages, persisted rebalance task format, or rebalance execution semantics.
This adds monitoring metrics. Their process-scoped lifetime and recovery behavior are documented.
Documentation
Add a Rebalance Metrics section to the monitoring documentation, describing metric names, types, meanings, and behavior during leadership changes and task recovery.