Jackie-Jiang commented on code in PR #13266:
URL: https://github.com/apache/pinot/pull/13266#discussion_r1621266450
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerTimer.java:
##########
@@ -27,7 +27,8 @@
*/
public enum ControllerTimer implements AbstractMetrics.Timer {
TABLE_REBALANCE_EXECUTION_TIME_MS("tableRebalanceExecutionTimeMs", false),
- CRON_SCHEDULER_JOB_EXECUTION_TIME_MS("cronSchedulerJobExecutionTimeMs",
false);
+ CRON_SCHEDULER_JOB_EXECUTION_TIME_MS("cronSchedulerJobExecutionTimeMs",
false),
+ IDEA_STATE_UPDATE_LATENCY("IdeaStateUpdateLatency", true);
Review Comment:
(minor)
```suggestion
IDEA_STATE_UPDATE_TIME_MS("IdeaStateUpdateTimeMs", true);
```
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java:
##########
@@ -64,7 +64,9 @@ public enum ControllerMeter implements AbstractMetrics.Meter {
TABLE_REBALANCE_FAILURE_DETECTED("TableRebalanceFailureDetected", false),
TABLE_REBALANCE_RETRY("TableRebalanceRetry", false),
TABLE_REBALANCE_RETRY_TOO_MANY_TIMES("TableRebalanceRetryTooManyTimes",
false),
- NUMBER_ADHOC_TASKS_SUBMITTED("adhocTasks", false);
+ NUMBER_ADHOC_TASKS_SUBMITTED("adhocTasks", false),
+ IDEA_STATE_UPDATE_FAILURE("IdeaStateUpdateError", true),
Review Comment:
We can track it for each resource. I feel it will be more useful than
tracking it globally
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java:
##########
@@ -99,11 +105,18 @@ public static IdealState cloneIdealState(IdealState
idealState) {
*/
public static IdealState updateIdealState(final HelixManager helixManager,
final String resourceName,
final Function<IdealState, IdealState> updater, RetryPolicy policy,
final boolean noChangeOk) {
+ final ControllerMetrics controllerMetrics = ControllerMetrics.get();
+ final AtomicInteger retryCounter = new AtomicInteger(-1);
try {
+ long updateStartTime = System.currentTimeMillis();
IdealStateWrapper idealStateWrapper = new IdealStateWrapper();
policy.attempt(new Callable<Boolean>() {
@Override
public Boolean call() {
+ retryCounter.incrementAndGet();
+ if (retryCounter.get() > 0) {
+
controllerMetrics.addMeteredGlobalValue(ControllerMeter.IDEA_STATE_UPDATE_RETRY,
1L);
+ }
Review Comment:
`policy.attempt()` should be able to return the retries
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java:
##########
@@ -99,11 +105,18 @@ public static IdealState cloneIdealState(IdealState
idealState) {
*/
public static IdealState updateIdealState(final HelixManager helixManager,
final String resourceName,
final Function<IdealState, IdealState> updater, RetryPolicy policy,
final boolean noChangeOk) {
+ final ControllerMetrics controllerMetrics = ControllerMetrics.get();
Review Comment:
(minor, convention) We don't usually use `final` for local variables
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java:
##########
@@ -64,7 +64,9 @@ public enum ControllerMeter implements AbstractMetrics.Meter {
TABLE_REBALANCE_FAILURE_DETECTED("TableRebalanceFailureDetected", false),
TABLE_REBALANCE_RETRY("TableRebalanceRetry", false),
TABLE_REBALANCE_RETRY_TOO_MANY_TIMES("TableRebalanceRetryTooManyTimes",
false),
- NUMBER_ADHOC_TASKS_SUBMITTED("adhocTasks", false);
+ NUMBER_ADHOC_TASKS_SUBMITTED("adhocTasks", false),
+ IDEA_STATE_UPDATE_FAILURE("IdeaStateUpdateError", true),
Review Comment:
(minor)
```suggestion
IDEA_STATE_UPDATE_FAILURE("IdeaStateUpdateFailure", true),
```
--
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]
