klsince commented on code in PR #11768:
URL: https://github.com/apache/pinot/pull/11768#discussion_r1355431764


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -241,13 +241,19 @@ public RebalanceResult rebalance(TableConfig tableConfig, 
RebalanceConfig rebala
           tierToInstancePartitionsMap, null);
     }
 
+    // Record the beginning of rebalance
+    
_tableRebalanceObserver.onTrigger(TableRebalanceObserver.Trigger.START_TRIGGER, 
currentAssignment,
+        targetAssignment);
+
     boolean segmentAssignmentUnchanged = 
currentAssignment.equals(targetAssignment);
     LOGGER.info("For rebalanceId: {}, instancePartitionsUnchanged: {}, 
tierInstancePartitionsUnchanged: {}, "
             + "segmentAssignmentUnchanged: {} for table: {}", rebalanceJobId, 
instancePartitionsUnchanged,
         tierInstancePartitionsUnchanged, segmentAssignmentUnchanged, 
tableNameWithType);
 
     if (segmentAssignmentUnchanged) {
-      LOGGER.info("Table: {} is already balanced", tableNameWithType);
+      String msg = String.format("Table: %s is already balanced", 
tableNameWithType);
+      LOGGER.info(msg);
+      _tableRebalanceObserver.onSuccess(msg);

Review Comment:
   > If you rebalance a table with "reassignAssignments" and the instance 
assignment doesn't change, you will get "IN_PROGRESS" back, but the id won't 
exist in zk 
   
   I'd assume we got a DONE status at L255 `if (dryRun) {...}` firstly (because 
rebalance restful API always does a dryRun firstly and triggers real trigger if 
dryRun gives DONE). But likely, the real table rebalance must have ended at the 
L259 `else {...}`. Because the observer.onTrigger(...START_TRIGGER) was run 
much later, so no job status got put in ZK for the jobId.
   
   If that's the case, then I'm wondering why we didn't return NO_NP for all 
three if branches below? cc @Jackie-Jiang 



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to