somandal commented on code in PR #16017:
URL: https://github.com/apache/pinot/pull/16017#discussion_r2136612615


##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/assignment/segment/StrictRealtimeSegmentAssignmentTest.java:
##########
@@ -255,9 +279,74 @@ public void testAssignSegmentWithOfflineSegment() {
     }
   }
 
-  @Test(expectedExceptions = IllegalStateException.class)
-  public void testAssignSegmentToCompletedServers() {
-    _segmentAssignment.assignSegment("seg01", new TreeMap<>(), new 
TreeMap<>());
+  @Test
+  public void testRebalanceDedupTableWithTiers() {
+    SegmentAssignment segmentAssignment = createSegmentAssignment("dedup");
+    Map<InstancePartitionsType, InstancePartitions> 
onlyConsumingInstancePartitionMap =
+        ImmutableMap.of(InstancePartitionsType.CONSUMING, 
_instancePartitionsMap.get(InstancePartitionsType.CONSUMING));
+    Map<String, Map<String, String>> currentAssignment = new TreeMap<>();
+    Set<String> segmentsOnTier = new HashSet<>();
+    for (int segmentId = 0; segmentId < 6; segmentId++) {
+      String segmentName = _segments.get(segmentId);
+      if (segmentId < 3) {
+        segmentsOnTier.add(segmentName);
+      }
+      List<String> instancesAssigned =
+          segmentAssignment.assignSegment(segmentName, currentAssignment, 
_instancePartitionsMap);
+      currentAssignment.put(segmentName,
+          SegmentAssignmentUtils.getInstanceStateMap(instancesAssigned, 
SegmentStateModel.ONLINE));
+    }
+    String tierName = "coldTier";
+    List<Tier> sortedTiers = createSortedTiers(tierName, segmentsOnTier);
+    Map<String, InstancePartitions> tierInstancePartitionsMap = 
createTierInstancePartitionsMap(tierName, 3);
+    RebalanceConfig rebalanceConfig = new RebalanceConfig();
+    rebalanceConfig.setIncludeConsuming(true);
+    Map<String, Map<String, String>> newAssignment =
+        segmentAssignment.rebalanceTable(currentAssignment, 
onlyConsumingInstancePartitionMap, sortedTiers,
+            tierInstancePartitionsMap, rebalanceConfig);
+    assertEquals(newAssignment.size(), currentAssignment.size());
+    for (String segName : currentAssignment.keySet()) {
+      if (segmentsOnTier.contains(segName)) {
+        assertTrue(newAssignment.get(segName).keySet().stream().allMatch(s -> 
s.startsWith(tierName)));
+      } else {
+        assertTrue(
+            newAssignment.get(segName).keySet().stream().allMatch(s -> 
s.startsWith(CONSUMING_INSTANCE_NAME_PREFIX)));
+      }
+    }
+  }
+
+  @Test(expectedExceptions = IllegalStateException.class, 
expectedExceptionsMessageRegExp = "Tiers must not be "
+      + "specified for table.*")
+  public void testRebalanceUpsertTableWithTiers() {
+    SegmentAssignment segmentAssignment = createSegmentAssignment("upsert");
+    Map<InstancePartitionsType, InstancePartitions> 
onlyConsumingInstancePartitionMap =
+        ImmutableMap.of(InstancePartitionsType.CONSUMING, 
_instancePartitionsMap.get(InstancePartitionsType.CONSUMING));
+    Map<String, Map<String, String>> currentAssignment = new TreeMap<>();
+    Set<String> segmentsOnTier = new HashSet<>();
+    for (int segmentId = 0; segmentId < 6; segmentId++) {
+      String segmentName = _segments.get(segmentId);
+      if (segmentId < 3) {
+        segmentsOnTier.add(segmentName);
+      }
+      List<String> instancesAssigned =
+          segmentAssignment.assignSegment(segmentName, currentAssignment, 
_instancePartitionsMap);
+      currentAssignment.put(segmentName,
+          SegmentAssignmentUtils.getInstanceStateMap(instancesAssigned, 
SegmentStateModel.ONLINE));
+    }
+
+    String tierName = "coldTier";
+    List<Tier> sortedTiers = createSortedTiers(tierName, segmentsOnTier);
+    Map<String, InstancePartitions> tierInstancePartitionsMap = 
createTierInstancePartitionsMap(tierName, 3);
+    RebalanceConfig rebalanceConfig = new RebalanceConfig();
+    rebalanceConfig.setIncludeConsuming(true);
+    segmentAssignment.rebalanceTable(currentAssignment, 
onlyConsumingInstancePartitionMap, sortedTiers,
+        tierInstancePartitionsMap, rebalanceConfig);
+  }
+
+  @Test(expectedExceptions = IllegalStateException.class, dataProvider = 
"tableTypes")
+  public void testAssignSegmentToCompletedServers(String tableType) {
+    SegmentAssignment segmentAssignment = createSegmentAssignment(tableType);
+    segmentAssignment.assignSegment("seg01", new TreeMap<>(), new TreeMap<>());

Review Comment:
   is this test ensuring that we don't move segments to "COMPLETED" instance 
partitions? but the passed in instancePartitionsMap is empty?
   
   Can we also have a test to reject if instancePartitionsMap has COMPLETED?
   
   perhaps also have similar test for rebalanceTables() API?



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