Cyrill commented on code in PR #6408:
URL: https://github.com/apache/ignite-3/pull/6408#discussion_r2283013924
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -957,18 +960,39 @@ private void onLogicalTopologyChanged(long term) {
// If the future is not here yet, this means we are still starting, so
learners will be updated after start
// (if we happen to become a leader).
+ if (serviceFuture == null) {
+ return;
+ }
- if (serviceFuture != null) {
- serviceFuture.thenCompose(service ->
service.isCurrentNodeLeader().thenCompose(isLeader -> {
- if (!isLeader) {
- return nullCompletedFuture();
- }
-
- return service.updateLearners(term);
- }));
+ synchronized (raftServiceLock) {
+ if (topologyReconfigurationFuture == null) {
+ topologyReconfigurationFuture =
+ serviceFuture.thenCompose(service ->
updateLearnersOnLeader(service, term));
+ } else {
+ topologyReconfigurationFuture =
+ // At the moment topology reconfiguration will stop if
the previous one fails (see how thenCompose works).
+ // This is going to change in the future when cmg/mg
majority loss is handled properly.
+ topologyReconfigurationFuture.thenCompose(v ->
+ serviceFuture.thenCompose(service ->
updateLearnersOnLeader(service, term))
+ );
+ }
}
}
+ private static CompletableFuture<Void>
updateLearnersOnLeader(CmgRaftService service, long term) {
Review Comment:
I read `maybeUpdateLearnersOnLeader` as "even we are on a leader, there is a
chance we are not willing to update learners".
So I would either stick to the current notation or change it to something
like "maybeUpdateLearners", which highlights the conditional nature of the body.
--
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
