yashmayya commented on code in PR #16096:
URL: https://github.com/apache/pinot/pull/16096#discussion_r2153702702
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -410,6 +410,11 @@ private RebalancePreCheckerResult
checkRebalanceConfig(RebalanceConfig rebalance
}
}
+ if (tableConfig.getTableType() == TableType.OFFLINE &&
rebalanceConfig.isForceCommitBeforeMoved()) {
+ pass = false;
+ warnings.add("forceCommitBeforeMoved is set for OFFLINE table, which
will be ignored.");
Review Comment:
Yeah +1, I'm not sure pre-checks is the best place to put this. I'd advocate
for either logging a warning and overriding it to `false` internally or
returning an error back to the user indicating that the option can't be used
with `OFFLINE` tables.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/ZkBasedTableRebalanceObserver.java:
##########
@@ -151,6 +151,24 @@ public void onTrigger(Trigger trigger, Map<String,
Map<String, String>> currentS
updatedStatsInZk = true;
}
break;
+ case FORCE_COMMIT_BEFORE_MOVED_START_TRIGGER:
+
_tableRebalanceProgressStats.getRebalanceProgressStatsOverall()._isForceCommittingConsumingSegments
= true;
+
_tableRebalanceProgressStats.getRebalanceProgressStatsCurrentStep()._isForceCommittingConsumingSegments
= true;
+ trackStatsInZk();
+ updatedStatsInZk = true;
+ break;
+ case FORCE_COMMIT_BEFORE_MOVED_END_TRIGGER:
+ LOGGER.info("force commit for consuming segments for table: {} is
done",
Review Comment:
Can you move this log to `TableRebalancer` as well for consistency? Also to
ensure that it is always logged regardless of the `TableRebalanceObserver`
implementation used.
--
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]
