sanpwc commented on code in PR #4446:
URL: https://github.com/apache/ignite-3/pull/4446#discussion_r1871246642


##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -1298,6 +1319,93 @@ private CompletableFuture<Boolean> 
onPrimaryExpired(PrimaryReplicaEventParameter
             return falseCompletedFuture();
         }
 
+        // TODO: move to Replica 
https://issues.apache.org/jira/browse/IGNITE-23750
+        private void registerFailoverCallback(TablePartitionId 
replicationGroupId) {
+            CompletableFuture<Replica> replicaFuture = 
replicaManager.replica(replicationGroupId);
+
+            // TODO: https://issues.apache.org/jira/browse/IGNITE-23753
+            // For now we assume that it's a possible situation to get null 
future on node's stopp process, but it should be fixed with
+            // a correct and consistent replica's state.
+            if (replicaFuture == null) {
+                return;
+            }
+
+            replicaFuture.thenApply(electedPrimaryReplica -> {

Review Comment:
   thenCompose in order to flat futures, WDYT?



##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -1298,6 +1319,93 @@ private CompletableFuture<Boolean> 
onPrimaryExpired(PrimaryReplicaEventParameter
             return falseCompletedFuture();
         }
 
+        // TODO: move to Replica 
https://issues.apache.org/jira/browse/IGNITE-23750
+        private void registerFailoverCallback(TablePartitionId 
replicationGroupId) {
+            CompletableFuture<Replica> replicaFuture = 
replicaManager.replica(replicationGroupId);
+
+            // TODO: https://issues.apache.org/jira/browse/IGNITE-23753
+            // For now we assume that it's a possible situation to get null 
future on node's stopp process, but it should be fixed with
+            // a correct and consistent replica's state.
+            if (replicaFuture == null) {
+                return;
+            }
+
+            replicaFuture.thenApply(electedPrimaryReplica -> {
+                onLeaderElectedFailoverCallback = (leaderNode, term) -> 
changePeersAndLearnersAsyncIfPendingExists(
+                        electedPrimaryReplica,
+                        replicationGroupId,
+                        term
+                );
+
+                return 
electedPrimaryReplica.raftClient().subscribeLeader(onLeaderElectedFailoverCallback);

Review Comment:
   Am I right that it worst case we will send changePeersAndLearners twice: on 
changePeersAndLearnersAsyncIfPendingExists and onLeaderElectedFailoverCallback?



##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -1298,6 +1319,93 @@ private CompletableFuture<Boolean> 
onPrimaryExpired(PrimaryReplicaEventParameter
             return falseCompletedFuture();
         }
 
+        // TODO: move to Replica 
https://issues.apache.org/jira/browse/IGNITE-23750
+        private void registerFailoverCallback(TablePartitionId 
replicationGroupId) {
+            CompletableFuture<Replica> replicaFuture = 
replicaManager.replica(replicationGroupId);
+
+            // TODO: https://issues.apache.org/jira/browse/IGNITE-23753
+            // For now we assume that it's a possible situation to get null 
future on node's stopp process, but it should be fixed with
+            // a correct and consistent replica's state.
+            if (replicaFuture == null) {
+                return;
+            }
+
+            replicaFuture.thenApply(electedPrimaryReplica -> {
+                onLeaderElectedFailoverCallback = (leaderNode, term) -> 
changePeersAndLearnersAsyncIfPendingExists(
+                        electedPrimaryReplica,
+                        replicationGroupId,
+                        term
+                );
+
+                return 
electedPrimaryReplica.raftClient().subscribeLeader(onLeaderElectedFailoverCallback);
+            }).exceptionally(e -> {
+                LOG.error("Rebalance failover subscription on elected primary 
replica failed [groupId=" + replicationGroupId + "].", e);
+

Review Comment:
   Should we trigger failHandler in that case?



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

Reply via email to