You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "sanpwc (via GitHub)" <gi...@apache.org> on 2025/06/13 06:59:25 UTC

[PR] IGNITE-25074 ItRebalanceDistributedTest adaptation to the colocaiton track. [ignite-3]

sanpwc opened a new pull request, #6036:
URL: https://github.com/apache/ignite-3/pull/6036

   https://issues.apache.org/jira/browse/IGNITE-25074


-- 
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-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-25074 ItRebalanceDistributedTest adaptation to the colocaiton track. [ignite-3]

Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6036:
URL: https://github.com/apache/ignite-3/pull/6036#discussion_r2151894452


##########
modules/placement-driver-api/src/testFixtures/java/org/apache/ignite/internal/placementdriver/TestPlacementDriver.java:
##########
@@ -127,19 +127,20 @@ public void setPrimaryReplicaSupplier(Supplier<? extends TestReplicaMetaImpl> pr
 
         TestReplicaMetaImpl replicaMeta = primaryReplicaSupplier.get();
 
-        fireEvent(
-                PrimaryReplicaEvent.PRIMARY_REPLICA_ELECTED,
-                new PrimaryReplicaEventParameters(
-                        // The only usage of causality token below is IndexBuildController that doesn't use in tests, so the actual value
-                        // doesn't matter there yet.
-                        0,
-                        replicaMeta.getReplicationGroupId(),
-                        replicaMeta.getLeaseholderId(),
-                        replicaMeta.getLeaseholder(),
-                        replicaMeta.getStartTime()
-                )
-        );
-
+        if (replicaMeta != null) {

Review Comment:
   Currently no, it's an artifact from my precious solution. Removed.



-- 
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-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-25074 ItRebalanceDistributedTest adaptation to the colocaiton track. [ignite-3]

Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #6036:
URL: https://github.com/apache/ignite-3/pull/6036#discussion_r2151573188


##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java:
##########
@@ -402,6 +399,19 @@ void before(TestInfo testInfo) throws Exception {
                 },
                 AWAIT_TIMEOUT_MILLIS
         ));
+
+        // Without default zone preparation we will catch "Critical system error" because test placement driver will return fake primary
+        // replica for default zone out of corresponding assignments.
+        if (colocationEnabled()) {
+            alterDefaultZone(node0);

Review Comment:
   We there alter default zone to replica factor 3 vs default 1. We want this because we don't want to wait for assignments and guess which one node is chosen for a partition assignment and then should be configured with test placement driver as primary, right?



##########
modules/placement-driver-api/src/testFixtures/java/org/apache/ignite/internal/placementdriver/TestPlacementDriver.java:
##########
@@ -127,19 +127,20 @@ public void setPrimaryReplicaSupplier(Supplier<? extends TestReplicaMetaImpl> pr
 
         TestReplicaMetaImpl replicaMeta = primaryReplicaSupplier.get();
 
-        fireEvent(
-                PrimaryReplicaEvent.PRIMARY_REPLICA_ELECTED,
-                new PrimaryReplicaEventParameters(
-                        // The only usage of causality token below is IndexBuildController that doesn't use in tests, so the actual value
-                        // doesn't matter there yet.
-                        0,
-                        replicaMeta.getReplicationGroupId(),
-                        replicaMeta.getLeaseholderId(),
-                        replicaMeta.getLeaseholder(),
-                        replicaMeta.getStartTime()
-                )
-        );
-
+        if (replicaMeta != null) {

Review Comment:
   Is there any case now when the supplier returns `null`?



-- 
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-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-25074 ItRebalanceDistributedTest adaptation to the colocaiton track. [ignite-3]

Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #6036:
URL: https://github.com/apache/ignite-3/pull/6036#discussion_r2151907174


##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java:
##########
@@ -402,6 +399,19 @@ void before(TestInfo testInfo) throws Exception {
                 },
                 AWAIT_TIMEOUT_MILLIS
         ));
+
+        // Without default zone preparation we will catch "Critical system error" because test placement driver will return fake primary
+        // replica for default zone out of corresponding assignments.
+        if (colocationEnabled()) {
+            alterDefaultZone(node0);

Review Comment:
   Nope.
   
   - Test placement driver will return some constant node as primary for all replication groups. Let's say it's node A.
   - In case of replica factor < NODES_COUNS (3 here in test) some partitions won't be located on that node A, because assignments will be either A or B in case of replicas 1, or AB in case of replicas 2.
   
   As a result "The local node is outside of the replication group" will be thrown because replica was not started on A (fake primary replica).



-- 
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-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-25074 ItRebalanceDistributedTest adaptation to the colocaiton track. [ignite-3]

Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc merged PR #6036:
URL: https://github.com/apache/ignite-3/pull/6036


-- 
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-unsubscribe@ignite.apache.org

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