You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/12/06 13:19:30 UTC
[GitHub] [ignite-3] rpuch opened a new pull request, #1412: IGNITE-18321 Return node to logical topology as soon as it gets returned to physical topology
rpuch opened a new pull request, #1412:
URL: https://github.com/apache/ignite-3/pull/1412
https://issues.apache.org/jira/browse/IGNITE-18321
--
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
[GitHub] [ignite-3] tkalkirill merged pull request #1412: IGNITE-18321 Return node to logical topology as soon as it gets returned to physical topology
Posted by GitBox <gi...@apache.org>.
tkalkirill merged PR #1412:
URL: https://github.com/apache/ignite-3/pull/1412
--
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
[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
Posted by 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
[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
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #1412:
URL: https://github.com/apache/ignite-3/pull/1412#discussion_r1043286853
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -125,6 +125,19 @@ public class ClusterManagementGroupManager implements IgniteComponent {
/** Handles cluster initialization flow. */
private final ClusterInitializer clusterInitializer;
+ /**
+ * Whether we attempted to complete join (i.e. send JoinReady command) on Ignite node start.
+ *
+ * <p>Such join completion always happens during a start, and it is always the last step during the startup process,
+ * to make sure a node joins the cluster when it's fully ready.
+ *
+ * <p>We need this flag to make sure we handle automatic rejoins correctly. If a short network hiccup happens, CMG leader
+ * might lose our node of sight, hence the node will be removed from physical and then from logical topologies. When
+ * the network connectivity is restored, the node will appear in the physical topology, after which it till try to
+ * rejoin the cluster. If such a 'rejoin' was carried unconditionally, it could happen before the first join during
+ * startup, so a not-yet-ready node could join the cluster. That's why we use this flag to only try such automatic
Review Comment:
I think that the "That's why" sentence can be removed, it's kinda redundant
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -125,6 +125,19 @@ public class ClusterManagementGroupManager implements IgniteComponent {
/** Handles cluster initialization flow. */
private final ClusterInitializer clusterInitializer;
+ /**
+ * Whether we attempted to complete join (i.e. send JoinReady command) on Ignite node start.
+ *
+ * <p>Such join completion always happens during a start, and it is always the last step during the startup process,
+ * to make sure a node joins the cluster when it's fully ready.
+ *
+ * <p>We need this flag to make sure we handle automatic rejoins correctly. If a short network hiccup happens, CMG leader
+ * might lose our node of sight, hence the node will be removed from physical and then from logical topologies. When
+ * the network connectivity is restored, the node will appear in the physical topology, after which it till try to
+ * rejoin the cluster. If such a 'rejoin' was carried unconditionally, it could happen before the first join during
Review Comment:
```suggestion
* rejoin the cluster. If such 'rejoin' was carried unconditionally, it could happen before the first join during
```
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -125,6 +125,19 @@ public class ClusterManagementGroupManager implements IgniteComponent {
/** Handles cluster initialization flow. */
private final ClusterInitializer clusterInitializer;
+ /**
+ * Whether we attempted to complete join (i.e. send JoinReady command) on Ignite node start.
+ *
+ * <p>Such join completion always happens during a start, and it is always the last step during the startup process,
+ * to make sure a node joins the cluster when it's fully ready.
+ *
+ * <p>We need this flag to make sure we handle automatic rejoins correctly. If a short network hiccup happens, CMG leader
+ * might lose our node of sight, hence the node will be removed from physical and then from logical topologies. When
+ * the network connectivity is restored, the node will appear in the physical topology, after which it till try to
Review Comment:
```suggestion
* the network connectivity is restored, the node will appear in the physical topology, after which it will try to
```
##########
modules/network/src/main/java/org/apache/ignite/network/DefaultMessagingService.java:
##########
@@ -404,7 +404,8 @@ public void stop() {
* 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.
+ * @param predicate Predicate that will decide whether a message should be dropped. Its first argument is the recipient
+ * node consistent ID.
Review Comment:
```suggestion
* node's consistent ID.
```
--
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
[GitHub] [ignite-3] rpuch commented on a diff in pull request #1412: IGNITE-18321 Return node to logical topology as soon as it gets returned to physical topology
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #1412:
URL: https://github.com/apache/ignite-3/pull/1412#discussion_r1043245516
##########
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:
Moved conditional join so that it is executed also in the case when CMG node set has changed
--
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