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