J-HowHuang commented on code in PR #16341:
URL: https://github.com/apache/pinot/pull/16341#discussion_r2211256763


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -1813,9 +1889,70 @@ public int fetch(String segmentName) {
     }
   }
 
+  @VisibleForTesting
+  @FunctionalInterface
+  interface DataLossRiskAssessor {
+    boolean hasDataLossRisk(String segmentName);
+  }
+
+  private static class DataLossRiskAssessorImpl implements 
DataLossRiskAssessor {
+    private final String _tableNameWithType;
+    private final TableConfig _tableConfig;
+    private final int _minAvailableReplicas;
+    private final HelixManager _helixManager;
+    private final PinotLLCRealtimeSegmentManager 
_pinotLLCRealtimeSegmentManager;
+    private final boolean _isPeerDownloadEnabled;
+    private final boolean _isPauselessEnabled;
+
+    private DataLossRiskAssessorImpl(String tableNameWithType, TableConfig 
tableConfig, int minAvailableReplicas,
+        HelixManager helixManager, PinotLLCRealtimeSegmentManager 
pinotLLCRealtimeSegmentManager) {
+      _tableNameWithType = tableNameWithType;
+      _tableConfig = tableConfig;
+      _minAvailableReplicas = minAvailableReplicas;
+      _helixManager = helixManager;
+      _pinotLLCRealtimeSegmentManager = pinotLLCRealtimeSegmentManager;
+      _isPeerDownloadEnabled = 
!StringUtils.isEmpty(tableConfig.getValidationConfig().getPeerSegmentDownloadScheme());
+      _isPauselessEnabled = 
PauselessConsumptionUtils.isPauselessEnabled(tableConfig);
+    }
+
+    @Override
+    public boolean hasDataLossRisk(String segmentName) {
+      // If peer-download is disabled, or minAvailableReplicas > 0 no data 
loss risk exists
+      if (!_isPeerDownloadEnabled || _minAvailableReplicas > 0) {
+        return false;
+      }

Review Comment:
   nit: since you have had `DataLossRiskAssessor` interface, can we have a 
`PeerDownloadTableDataLossRiskAssessorImpl` and a 
`NoopDataLossRiskAssessorImpl` for example, which would be easier to maintain 
and read if we're adding more assessment like this in the future?



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