This is an automated email from the ASF dual-hosted git repository.
sdanilov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new c9bff85d94 IGNITE-19420 Remove a node from Logical Topology as soon
as it leaves Physical Topology (#2026)
c9bff85d94 is described below
commit c9bff85d94b9cd0fcc6eb3812bf32e1511bf8afe
Author: Roman Puchkovskiy <[email protected]>
AuthorDate: Mon May 8 10:09:11 2023 +0400
IGNITE-19420 Remove a node from Logical Topology as soon as it leaves
Physical Topology (#2026)
---
.../internal/cli/CliIntegrationTestBase.java | 5 +--
.../call/CallInitializedIntegrationTestBase.java | 2 +-
.../management/ClusterManagementGroupManager.java | 12 +-----
.../ClusterManagementConfigurationSchema.java | 8 ----
.../management/BaseItClusterManagementTest.java | 4 +-
.../impl/ItMetaStorageMultipleNodesTest.java | 2 +-
.../ignite/internal/rest/cluster/RestTestBase.java | 2 +-
.../internal/ClusterPerTestIntegrationTest.java | 32 +++-----------
.../internal/compute/ItLogicalTopologyTest.java | 49 ++++++++++------------
9 files changed, 36 insertions(+), 80 deletions(-)
diff --git
a/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/CliIntegrationTestBase.java
b/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/CliIntegrationTestBase.java
index 286ca728b8..df3508ef39 100644
---
a/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/CliIntegrationTestBase.java
+++
b/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/CliIntegrationTestBase.java
@@ -43,7 +43,7 @@ import org.junit.jupiter.api.TestInstance;
*/
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@MicronautTest(rebuildContext = true)
-public class CliIntegrationTestBase extends IntegrationTestBase {
+public abstract class CliIntegrationTestBase extends IntegrationTestBase {
/**
* Template for node bootstrap config with Scalecube and Logical Topology
settings for fast failure detection.
*/
@@ -62,8 +62,7 @@ public class CliIntegrationTestBase extends
IntegrationTestBase {
+ " gossipInterval: 10\n"
+ " },\n"
+ " }\n"
- + " },"
- + " cluster.failoverTimeout: 100\n"
+ + " }\n"
+ "}";
diff --git
a/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/call/CallInitializedIntegrationTestBase.java
b/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/call/CallInitializedIntegrationTestBase.java
index 7f6ee95d3d..eab6b86a97 100644
---
a/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/call/CallInitializedIntegrationTestBase.java
+++
b/modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/call/CallInitializedIntegrationTestBase.java
@@ -28,7 +28,7 @@ import org.junit.jupiter.api.TestInfo;
/**
* Base class for call integration tests that needs initialized ignite
cluster. Contains common methods and useful assertions.
*/
-public class CallInitializedIntegrationTestBase extends CliIntegrationTestBase
{
+public abstract class CallInitializedIntegrationTestBase extends
CliIntegrationTestBase {
protected UrlCallInput urlInput;
@BeforeAll
diff --git
a/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java
b/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java
index 93a4bf7352..4b737de7bc 100644
---
a/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java
+++
b/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java
@@ -634,21 +634,11 @@ public class ClusterManagementGroupManager implements
IgniteComponent {
@Override
public void onDisappeared(ClusterNode member) {
- scheduleRemoveFromLogicalTopology(raftService, member);
+ raftService.removeFromCluster(Set.of(member));
}
};
}
- private void scheduleRemoveFromLogicalTopology(CmgRaftService raftService,
ClusterNode node) {
- scheduledExecutor.schedule(() -> {
- ClusterNode physicalTopologyNode =
clusterService.topologyService().getByConsistentId(node.name());
-
- if (physicalTopologyNode == null ||
!physicalTopologyNode.id().equals(node.id())) {
- raftService.removeFromCluster(Set.of(node));
- }
- }, configuration.failoverTimeout().value(), TimeUnit.MILLISECONDS);
- }
-
private void sendClusterState(CmgRaftService raftService, ClusterNode
node) {
sendClusterState(raftService, List.of(node));
}
diff --git
a/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/configuration/ClusterManagementConfigurationSchema.java
b/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/configuration/ClusterManagementConfigurationSchema.java
index 7482bbed5c..33a9ab64db 100644
---
a/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/configuration/ClusterManagementConfigurationSchema.java
+++
b/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/configuration/ClusterManagementConfigurationSchema.java
@@ -17,7 +17,6 @@
package org.apache.ignite.internal.cluster.management.configuration;
-import java.util.concurrent.TimeUnit;
import org.apache.ignite.configuration.annotation.ConfigurationRoot;
import org.apache.ignite.configuration.annotation.ConfigurationType;
import org.apache.ignite.configuration.annotation.Value;
@@ -32,11 +31,4 @@ public class ClusterManagementConfigurationSchema {
@Value(hasDefault = true)
@Range(min = 1)
public long networkInvokeTimeout = 500;
-
- /**
- * Delay between a moment a node drops out from the physical topology and
when it gets removed from the logical topology (ms).
- */
- @Value(hasDefault = true)
- @Range(min = 0)
- public long failoverTimeout = TimeUnit.SECONDS.toMillis(30);
}
diff --git
a/modules/cluster-management/src/testFixtures/java/org/apache/ignite/internal/cluster/management/BaseItClusterManagementTest.java
b/modules/cluster-management/src/testFixtures/java/org/apache/ignite/internal/cluster/management/BaseItClusterManagementTest.java
index 9d2ba11566..0750ebb6fc 100644
---
a/modules/cluster-management/src/testFixtures/java/org/apache/ignite/internal/cluster/management/BaseItClusterManagementTest.java
+++
b/modules/cluster-management/src/testFixtures/java/org/apache/ignite/internal/cluster/management/BaseItClusterManagementTest.java
@@ -43,10 +43,10 @@ import org.junit.jupiter.api.extension.ExtendWith;
* Base class for integration tests that use a cluster of {@link MockNode}s.
*/
@ExtendWith(ConfigurationExtension.class)
-public class BaseItClusterManagementTest extends BaseIgniteAbstractTest {
+public abstract class BaseItClusterManagementTest extends
BaseIgniteAbstractTest {
private static final int PORT_BASE = 10000;
- @InjectConfiguration("mock.failoverTimeout: 100")
+ @InjectConfiguration
private static ClusterManagementConfiguration cmgConfiguration;
@InjectConfiguration
diff --git
a/modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageMultipleNodesTest.java
b/modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageMultipleNodesTest.java
index a094d4f880..5193d59a95 100644
---
a/modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageMultipleNodesTest.java
+++
b/modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageMultipleNodesTest.java
@@ -95,7 +95,7 @@ public class ItMetaStorageMultipleNodesTest extends
IgniteAbstractTest {
@InjectConfiguration
private static RaftConfiguration raftConfiguration;
- @InjectConfiguration("mock.failoverTimeout=0")
+ @InjectConfiguration
private static ClusterManagementConfiguration cmgConfiguration;
@InjectConfiguration
diff --git
a/modules/rest/src/integrationTest/java/org/apache/ignite/internal/rest/cluster/RestTestBase.java
b/modules/rest/src/integrationTest/java/org/apache/ignite/internal/rest/cluster/RestTestBase.java
index e6994bb7f7..bfefa4067f 100644
---
a/modules/rest/src/integrationTest/java/org/apache/ignite/internal/rest/cluster/RestTestBase.java
+++
b/modules/rest/src/integrationTest/java/org/apache/ignite/internal/rest/cluster/RestTestBase.java
@@ -41,7 +41,7 @@ import org.junit.jupiter.api.extension.ExtendWith;
*/
@MicronautTest
@ExtendWith(WorkDirectoryExtension.class)
-public class RestTestBase extends BaseItClusterManagementTest {
+public abstract class RestTestBase extends BaseItClusterManagementTest {
static final List<MockNode> cluster = new ArrayList<>();
static ClusterService clusterService;
diff --git
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/ClusterPerTestIntegrationTest.java
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/ClusterPerTestIntegrationTest.java
index 3a38180d73..059c26d582 100644
---
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/ClusterPerTestIntegrationTest.java
+++
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/ClusterPerTestIntegrationTest.java
@@ -19,8 +19,6 @@ package org.apache.ignite.internal;
import static
org.apache.ignite.internal.sql.engine.util.CursorUtils.getAllFromCursor;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
import java.nio.file.Path;
import java.util.List;
import org.apache.ignite.Ignite;
@@ -38,7 +36,6 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.api.Timeout;
-import org.junit.platform.commons.support.ReflectionSupport;
/**
* Abstract integration test that starts and stops a cluster per test method.
@@ -76,8 +73,7 @@ public abstract class ClusterPerTestIntegrationTest extends
IgniteIntegrationTes
+ " gossipInterval: 10\n"
+ " },\n"
+ " }\n"
- + " },"
- + " cluster.failoverTimeout: 100\n"
+ + " }\n"
+ "}";
protected Cluster cluster;
@@ -95,6 +91,10 @@ public abstract class ClusterPerTestIntegrationTest extends
IgniteIntegrationTes
@BeforeEach
public void setup(TestInfo testInfo) throws Exception {
setupBase(testInfo, workDir);
+
+ cluster = new Cluster(testInfo, workDir,
getNodeBootstrapConfigTemplate());
+
+ cluster.startAndInit(initialNodes());
}
/**
@@ -104,30 +104,10 @@ public abstract class ClusterPerTestIntegrationTest
extends IgniteIntegrationTes
* @throws Exception If failed.
*/
@AfterEach
+ @Timeout(60)
public void tearDown(TestInfo testInfo) throws Exception {
tearDownBase(testInfo);
- }
-
- @BeforeEach
- void startAndInitCluster(TestInfo testInfo) {
- cluster = new Cluster(testInfo, workDir,
getNodeBootstrapConfigTemplate());
- cluster.startAndInit(initialNodes());
- }
-
- private String invokeArglessMethod(Class<?> testClass, String methodName) {
- Method method = ReflectionSupport.findMethod(testClass,
methodName).orElseThrow();
-
- if (!Modifier.isStatic(method.getModifiers())) {
- throw new IllegalStateException(methodName + " is expected to be
static");
- }
-
- return (String) ReflectionSupport.invokeMethod(method, null);
- }
-
- @AfterEach
- @Timeout(60)
- void shutdownCluster() {
cluster.shutdown();
}
diff --git
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/compute/ItLogicalTopologyTest.java
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/compute/ItLogicalTopologyTest.java
index ad323a3463..e053ae4cf4 100644
---
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/compute/ItLogicalTopologyTest.java
+++
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/compute/ItLogicalTopologyTest.java
@@ -23,7 +23,6 @@ import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -41,10 +40,10 @@ import org.apache.ignite.Ignite;
import org.apache.ignite.IgnitionManager;
import org.apache.ignite.internal.ClusterPerTestIntegrationTest;
import org.apache.ignite.internal.app.IgniteImpl;
-import
org.apache.ignite.internal.cluster.management.configuration.ClusterManagementConfiguration;
import org.apache.ignite.internal.cluster.management.topology.api.LogicalNode;
import
org.apache.ignite.internal.cluster.management.topology.api.LogicalTopologyEventListener;
import
org.apache.ignite.internal.cluster.management.topology.api.LogicalTopologySnapshot;
+import org.apache.ignite.internal.network.configuration.NetworkConfiguration;
import org.apache.ignite.internal.network.message.ScaleCubeMessage;
import org.apache.ignite.internal.tostring.S;
import org.intellij.lang.annotations.Language;
@@ -72,8 +71,7 @@ class ItLogicalTopologyTest extends
ClusterPerTestIntegrationTest {
+ " },"
+ " nodeAttributes: {\n"
+ " nodeAttributes: " + NODE_ATTRIBUTES
- + " },\n"
- + " cluster.failoverTimeout: 100\n"
+ + " }\n"
+ "}";
private final LogicalTopologyEventListener listener = new
LogicalTopologyEventListener() {
@@ -280,23 +278,21 @@ class ItLogicalTopologyTest extends
ClusterPerTestIntegrationTest {
}
@Test
- void nodeDoesNotLeaveLogicalTopologyImmediatelyAfterBeingLostBySwim()
throws Exception {
+ void nodeLeavesLogicalTopologyImmediatelyAfterBeingLostBySwim() throws
Exception {
IgniteImpl entryNode = node(0);
- setInfiniteClusterFailoverTimeout(entryNode);
-
- startNode(1);
+ IgniteImpl secondNode = startNode(1);
entryNode.logicalTopologyService().addEventListener(listener);
// Knock the node out without allowing it to say good bye.
cluster.simulateNetworkPartitionOf(1);
- // 1 second is usually insufficient on my machine to get an event,
even if it's produced. On the CI we
- // should probably account for spurious delays due to other processes,
hence 2 seconds.
- Event event = events.poll(2, TimeUnit.SECONDS);
+ Event leaveEvent = events.poll(10, TimeUnit.SECONDS);
- assertThat(event, is(nullValue()));
+ assertThat(leaveEvent, is(notNullValue()));
+ assertThat(leaveEvent.node.name(), is(secondNode.name()));
+ assertThat(leaveEvent.eventType, is(EventType.LEFT));
}
@Test
@@ -314,9 +310,6 @@ class ItLogicalTopologyTest extends
ClusterPerTestIntegrationTest {
}
});
- // Disable the failover timeout so that the CMG Raft group is
immediately notified of a node leaving Physical Topology.
- setZeroClusterFailoverTimeout(entryNode);
-
cluster.startClusterNode(1);
try {
@@ -338,9 +331,11 @@ class ItLogicalTopologyTest extends
ClusterPerTestIntegrationTest {
}
@Test
- void nodeLeavesLogicalTopologyOnStop() throws Exception {
+ void nodeLeavesLogicalTopologyImmediatelyOnGracefulStop() throws Exception
{
IgniteImpl entryNode = node(0);
+ disableNetworkFailureDetection(entryNode);
+
IgniteImpl secondIgnite = startNode(1);
entryNode.logicalTopologyService().addEventListener(listener);
@@ -357,17 +352,17 @@ class ItLogicalTopologyTest extends
ClusterPerTestIntegrationTest {
assertThat(leaveEvent.node.name(), is(secondIgnite.name()));
}
- private static void setInfiniteClusterFailoverTimeout(IgniteImpl node)
throws Exception {
-
node.nodeConfiguration().getConfiguration(ClusterManagementConfiguration.KEY)
- .failoverTimeout()
- .update(Long.MAX_VALUE)
- .get(10, TimeUnit.SECONDS);
- }
-
- private static void setZeroClusterFailoverTimeout(IgniteImpl node) throws
Exception {
-
node.nodeConfiguration().getConfiguration(ClusterManagementConfiguration.KEY)
- .failoverTimeout()
- .update(0L)
+ /**
+ * Configures the given Ignite instance in a way that makes it impossible
to notice (by itself) that another
+ * node has gone and is not reachable on the network anymore.
+ *
+ * @param node Ignite node to disable network failure detection at.
+ * @throws Exception If something goes wrong.
+ */
+ private static void disableNetworkFailureDetection(IgniteImpl node) throws
Exception {
+ node.nodeConfiguration().getConfiguration(NetworkConfiguration.KEY)
+ .membership()
+ .change(membershipChange ->
membershipChange.changeFailurePingInterval(Integer.MAX_VALUE))
.get(10, TimeUnit.SECONDS);
}
