zpinto commented on code in PR #2986: URL: https://github.com/apache/helix/pull/2986#discussion_r1924255760
########## helix-rest/src/main/java/org/apache/helix/rest/server/service/VirtualTopologyGroupService.java: ########## @@ -60,15 +62,15 @@ public class VirtualTopologyGroupService { private final ClusterService _clusterService; private final ConfigAccessor _configAccessor; private final HelixDataAccessor _dataAccessor; - private final VirtualGroupAssignmentAlgorithm _assignmentAlgorithm; + private VirtualGroupAssignmentAlgorithm _assignmentAlgorithm; public VirtualTopologyGroupService(HelixAdmin helixAdmin, ClusterService clusterService, ConfigAccessor configAccessor, HelixDataAccessor dataAccessor) { _helixAdmin = helixAdmin; _clusterService = clusterService; _configAccessor = configAccessor; _dataAccessor = dataAccessor; - _assignmentAlgorithm = FifoVirtualGroupAssignmentAlgorithm.getInstance(); + _assignmentAlgorithm = FaultZoneBasedVirtualGroupAssignmentAlgorithm.getInstance(); // default assignment algorithm Review Comment: We should not change the algorithm default or this will break backwards compatibility. ########## helix-core/src/main/java/org/apache/helix/cloud/topology/VirtualGroupAssignmentAlgorithm.java: ########## @@ -34,5 +34,5 @@ public interface VirtualGroupAssignmentAlgorithm { * @return the assignment as mapping from virtual group ID to instanceIds */ Map<String, Set<String>> computeAssignment(int numGroups, String virtualGroupName, - Map<String, Set<String>> zoneMapping); + Map<String, Set<String>> zoneMapping, Map<String, Set<String>> virtualZoneMapping); Review Comment: Let's add a new signature with default implementation: Ex: ``` default Map<String, Set<String>> computeAssignment(int numGroups, String virtualGroupName, Map<String, Set<String>> zoneMapping, Map<String, Set<String>> virtualZoneMapping) { omputeAssignment(int numGroups, String virtualGroupName, Map<String, Set<String>> zoneMapping); } ``` -- 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: reviews-unsubscr...@helix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org