Jackie-Jiang commented on code in PR #13266:
URL: https://github.com/apache/pinot/pull/13266#discussion_r1621469520


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java:
##########
@@ -120,7 +127,6 @@ public Boolean call() {
             LOGGER.error("Caught permanent exception while updating ideal 
state for resource: {}", resourceName, e);
             throw e;
           } catch (Exception e) {
-            LOGGER.error("Caught exception while updating ideal state for 
resource: {}", resourceName, e);

Review Comment:
   Suggest keeping this error log. It is unexpected



##########
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java:
##########
@@ -99,9 +104,11 @@ 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) {
+    ControllerMetrics controllerMetrics = ControllerMetrics.get();
     try {
+      long updateStartTime = System.currentTimeMillis();

Review Comment:
   (nit)
   ```suggestion
         long startTimeMs = System.currentTimeMillis();
   ```



-- 
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]

Reply via email to