Jackie-Jiang commented on code in PR #19784:
URL: https://github.com/apache/pinot/pull/19784#discussion_r4252771726
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/BrokerMeter.java:
##########
@@ -201,6 +201,20 @@ public class BrokerMeter implements AbstractMetrics.Meter {
public static final BrokerMeter SERVER_MISSING_FOR_ROUTING = create(
"SERVER_MISSING_FOR_ROUTING", "badResponses", false);
+ /// Number of times the broker updated the routing of a table after a change
to its ideal state or external view.
+ /// Failed updates are counted in [#ROUTING_TABLE_UPDATE_FAILURES] instead.
+ ///
+ /// This metric is not global and is attached to a particular table.
+ public static final BrokerMeter ROUTING_TABLE_UPDATES =
create("ROUTING_TABLE_UPDATES", "updates", false);
Review Comment:
Naming nit: consider `ROUTING_TABLE_UPDATE_SUCCESS` paired with
`ROUTING_TABLE_UPDATE_FAILURE` to make the success-only count explicit. The
singular suffixes follow existing pairs such as `IDEAL_STATE_UPDATE_SUCCESS` /
`IDEAL_STATE_UPDATE_FAILURE` and `SEGMENT_UPLOAD_SUCCESS` /
`SEGMENT_UPLOAD_FAILURE`.
##########
pinot-broker/src/test/java/org/apache/pinot/broker/routing/manager/BrokerRoutingManagerTest.java:
##########
@@ -573,6 +576,52 @@ public void testRemoveRoutingRemovesReplicaHealthMetrics()
_routingManager.removeRouting(TEST_TABLE);
verifyReplicaHealthGaugesRemoved();
+ verify(_brokerMetrics).removeTableGauge(TEST_TABLE,
BrokerGauge.ROUTING_TABLE_SEGMENTS);
+ }
+
+ @Test
+ public void testAssignmentChangeReportsRebuildMetrics()
Review Comment:
Naming nit: consider `testAssignmentChangeReportsRoutingUpdateMetrics`. This
tests assignment-change updates; full routing builds are a separate path and do
not emit the timer or success counter.
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/manager/BaseBrokerRoutingManager.java:
##########
@@ -1427,12 +1446,15 @@ private static class RoutingEntry {
transient boolean _disabled;
+ // Number of pre-selected online segments at the last build or assignment
change
+ transient int _numSegments;
Review Comment:
Naming nit: consider `_numPreSelectedOnlineSegments` and
`getNumPreSelectedOnlineSegments()` to identify the exact population being
counted. This is the preselected ideal-state population, and the value is
updated even if a subsequent routing component update fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
