J-HowHuang commented on code in PR #16722:
URL: https://github.com/apache/pinot/pull/16722#discussion_r2311170635
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/relocation/SegmentRelocator.java:
##########
@@ -182,34 +183,46 @@ BlockingQueue<String> getWaitingQueue() {
return _waitingQueue;
}
- private void rebalanceTable(String tableNameWithType) {
+ @VisibleForTesting
+ void rebalanceTable(String tableNameWithType) {
TableConfig tableConfig =
_pinotHelixResourceManager.getTableConfig(tableNameWithType);
Preconditions.checkState(tableConfig != null, "Failed to find table config
for table: %s", tableNameWithType);
+ RebalanceConfig rebalanceConfig = new RebalanceConfig();
+
boolean relocate = false;
- if (TierConfigUtils.shouldRelocateToTiers(tableConfig)) {
- relocate = true;
- LOGGER.info("Relocating segments to tiers for table: {}",
tableNameWithType);
- }
if (tableConfig.getTableType() == TableType.REALTIME
&&
InstanceAssignmentConfigUtils.shouldRelocateCompletedSegments(tableConfig)) {
relocate = true;
LOGGER.info("Relocating COMPLETED segments for table: {}",
tableNameWithType);
}
+ if (TierConfigUtils.shouldRelocateToTiers(tableConfig)) {
+ relocate = true;
+ rebalanceConfig.setUpdateTargetTier(true);
+ LOGGER.info("Relocating segments to tiers for table: {}",
tableNameWithType);
+ } else {
+ List<SegmentZKMetadata> segmentsZKMetadata =
_pinotHelixResourceManager.getSegmentsZKMetadata(tableNameWithType);
Review Comment:
> a) limit the check to only tables that may have had tiered storage enabled
Segment ZK metadata is the only thing we persist tier info afaik. If we want
we may come up with some other way to store the table-level tier info in ZK.
But consider this is a periodic job with default frequency 1hr, I think it's
fine to have this. Plus other periodic task such as `RetentionManager` also
fetches the full segment ZK metadata list to purge segments, I think it's fair
to use `getSegmentsZKMetadata` in this context.
https://github.com/apache/pinot/blob/850b6ac28214e5493bbe004409d49675402d74f1/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java#L167-L169
> b) have a way to check only a subset of segments
Maybe we can do sampling, but if the cost of calling `getSegmentsZKMetadata`
is not a big problem I think it'd be a bit overkill
--
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]
