[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1412: IGNITE-18321 Return node to logical topology as soon as it gets returned to physical topology

[GitBox <gi...@apache.org>]

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


##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -125,6 +125,8 @@ public class ClusterManagementGroupManager implements IgniteComponent {
     /** Handles cluster initialization flow. */
     private final ClusterInitializer clusterInitializer;
 
+    private volatile boolean attemptedCompleteJoinOnStart = false;

Review Comment:
   Please add some javadocs about why this flag is needed



##########
modules/network/src/main/java/org/apache/ignite/network/DefaultMessagingService.java:
##########
@@ -389,4 +399,25 @@ public void stop() {
         IgniteUtils.shutdownAndAwaitTermination(inboundExecutor, 10, TimeUnit.SECONDS);
         IgniteUtils.shutdownAndAwaitTermination(outboundExecutor, 10, TimeUnit.SECONDS);
     }
+
+    /**
+     * Installs a predicate, it will be consulted with for each message being sent; when it returns {@code true}, the
+     * message will be silently dropped (it will not be sent, the corresponding future will never complete).
+     *
+     * @param predicate Predicate that will decide whether a message should be dropped.
+     */
+    @TestOnly
+    public void dropMessages(BiPredicate<String, NetworkMessage> predicate) {

Review Comment:
   Please document what the first parameter of the predicate means or use an interface



##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -420,7 +422,15 @@ private void handleClusterState(ClusterStateMessage msg, String senderConsistent
                             if (service != null && service.nodeNames().equals(state.cmgNodes())) {
                                 LOG.info("ClusterStateMessage received, but the CMG service is already started");
 
-                                return completedFuture(service);
+                                return joinCluster(service, state.clusterTag())

Review Comment:
   Why did you place this code here? What if the CMG nodes changed and we need to re-create the service?



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