Re: [PR] IGNITE-22624 Fix ItEstimatedSizeTest in case of enabled colocation. [ignite-3]

[via GitHub]

sashapolo commented on code in PR #5607:
URL: https://github.com/apache/ignite-3/pull/5607#discussion_r2039709042


##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/table/ItEstimatedSizeTest.java:
##########
@@ -178,23 +183,36 @@ private void waitForRebalance(int numNodes) throws 
InterruptedException {
         }
     }
 
-    private Set<String> stableAssignmentNodes() {
+    private Set<String> stableAssignmentNodes(int zoneId) {
         MetaStorageManager metaStorageManager = 
unwrapIgniteImpl(cluster.aliveNode()).metaStorageManager();
 
         var stableAssignmentsPrefix = enabledColocation()
                 ? new 
ByteArray(ZoneRebalanceUtil.STABLE_ASSIGNMENTS_PREFIX_BYTES)
                 : new ByteArray(STABLE_ASSIGNMENTS_PREFIX_BYTES);
 
+

Review Comment:
   ?



##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/table/ItEstimatedSizeTest.java:
##########
@@ -115,7 +116,9 @@ void testEstimatedSizeAfterScaleUp() throws 
InterruptedException {
         cluster.startNode(initialNodes());
 
         for (String profile : ALL_STORAGE_PROFILES) {
-            waitForRebalance(initialNodes() + 1);
+            int zoneId = 
unwrapTableImpl(unwrapIgniteImpl(cluster.aliveNode()).tables().table(tableName(profile))).zoneId();

Review Comment:
   Please extract this into a method



##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/table/ItEstimatedSizeTest.java:
##########
@@ -178,23 +183,36 @@ private void waitForRebalance(int numNodes) throws 
InterruptedException {
         }
     }
 
-    private Set<String> stableAssignmentNodes() {
+    private Set<String> stableAssignmentNodes(int zoneId) {
         MetaStorageManager metaStorageManager = 
unwrapIgniteImpl(cluster.aliveNode()).metaStorageManager();
 
         var stableAssignmentsPrefix = enabledColocation()
                 ? new 
ByteArray(ZoneRebalanceUtil.STABLE_ASSIGNMENTS_PREFIX_BYTES)
                 : new ByteArray(STABLE_ASSIGNMENTS_PREFIX_BYTES);
 
+
         CompletableFuture<List<Entry>> entriesFuture = 
subscribeToList(metaStorageManager.prefix(stableAssignmentsPrefix));
 
         assertThat(entriesFuture, willCompleteSuccessfully());
 
-        return entriesFuture.join().stream()
-                .map(entry -> Assignments.fromBytes(entry.value()))
-                .filter(Objects::nonNull)
-                .flatMap(assignments -> assignments.nodes().stream())
-                .map(Assignment::consistentId)
-                .collect(toSet());
+        if (enabledColocation()) {
+            return entriesFuture.join().stream()
+                    .filter(entry ->
+                            
ZoneRebalanceUtil.extractZonePartitionId(entry.key(), 
ZoneRebalanceUtil.STABLE_ASSIGNMENTS_PREFIX_BYTES)

Review Comment:
   Let's introduce some static imports here



##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/table/ItEstimatedSizeTest.java:
##########
@@ -178,23 +183,36 @@ private void waitForRebalance(int numNodes) throws 
InterruptedException {
         }
     }
 
-    private Set<String> stableAssignmentNodes() {
+    private Set<String> stableAssignmentNodes(int zoneId) {
         MetaStorageManager metaStorageManager = 
unwrapIgniteImpl(cluster.aliveNode()).metaStorageManager();
 
         var stableAssignmentsPrefix = enabledColocation()
                 ? new 
ByteArray(ZoneRebalanceUtil.STABLE_ASSIGNMENTS_PREFIX_BYTES)
                 : new ByteArray(STABLE_ASSIGNMENTS_PREFIX_BYTES);
 
+
         CompletableFuture<List<Entry>> entriesFuture = 
subscribeToList(metaStorageManager.prefix(stableAssignmentsPrefix));
 
         assertThat(entriesFuture, willCompleteSuccessfully());
 
-        return entriesFuture.join().stream()
-                .map(entry -> Assignments.fromBytes(entry.value()))
-                .filter(Objects::nonNull)
-                .flatMap(assignments -> assignments.nodes().stream())
-                .map(Assignment::consistentId)
-                .collect(toSet());
+        if (enabledColocation()) {
+            return entriesFuture.join().stream()
+                    .filter(entry ->
+                            
ZoneRebalanceUtil.extractZonePartitionId(entry.key(), 
ZoneRebalanceUtil.STABLE_ASSIGNMENTS_PREFIX_BYTES)

Review Comment:
   Can we use a prefix that already contains the zone ID instead of manually 
filtering all stable assignments?



-- 
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: notifications-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org