valepakh commented on code in PR #4614:
URL: https://github.com/apache/ignite-3/pull/4614#discussion_r1824144594
##########
modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/cluster/ClusterState.java:
##########
@@ -64,24 +73,28 @@ public class ClusterState {
@JsonCreator
public ClusterState(
@JsonProperty("cmgNodes") Collection<String> cmgNodes,
- @JsonProperty("msNodes") Collection<String> msNodes,
+ @JsonProperty("msNodes") Collection<String> msNodes,
Review Comment:
```suggestion
@JsonProperty("msNodes") Collection<String> msNodes,
```
##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/sql/ItSqlCommandTest.java:
##########
@@ -33,6 +33,8 @@ class ItSqlCommandTest extends CliSqlCommandTestBase {
void nonExistingFile() {
execute("sql", "--file", "nonexisting", "--jdbc-url", JDBC_URL);
+ CLUSTER.stopNode(0);
Review Comment:
Why this is needed?
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/cluster/ClusterManagementController.java:
##########
@@ -84,16 +113,31 @@ public CompletableFuture<Void> init(@Body InitCommand
initCommand) {
});
}
- private static ClusterState
mapClusterState(org.apache.ignite.internal.cluster.management.ClusterState
clusterState) {
+ private ClusterState
mapClusterState(org.apache.ignite.internal.cluster.management.ClusterState
clusterState) {
return new ClusterState(
clusterState.cmgNodes(),
clusterState.metaStorageNodes(),
clusterState.igniteVersion().toString(),
new ClusterTag(clusterState.clusterTag().clusterName(),
clusterState.clusterTag().clusterId()),
- clusterState.formerClusterIds()
+ clusterState.formerClusterIds(),
+ mapClusterStatus(clusterState)
);
}
+ private ClusterStatus
mapClusterStatus(org.apache.ignite.internal.cluster.management.ClusterState
clusterState) {
+ Set<String> metaStorageNodes = clusterState.metaStorageNodes();
+ long presentedMetaStorageNodes = topologyService.allMembers().stream()
Review Comment:
```suggestion
long presentMetaStorageNodes = topologyService.allMembers().stream()
```
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/cluster/ClusterManagementController.java:
##########
@@ -84,16 +113,31 @@ public CompletableFuture<Void> init(@Body InitCommand
initCommand) {
});
}
- private static ClusterState
mapClusterState(org.apache.ignite.internal.cluster.management.ClusterState
clusterState) {
+ private ClusterState
mapClusterState(org.apache.ignite.internal.cluster.management.ClusterState
clusterState) {
return new ClusterState(
clusterState.cmgNodes(),
clusterState.metaStorageNodes(),
clusterState.igniteVersion().toString(),
new ClusterTag(clusterState.clusterTag().clusterName(),
clusterState.clusterTag().clusterId()),
- clusterState.formerClusterIds()
+ clusterState.formerClusterIds(),
+ mapClusterStatus(clusterState)
);
}
+ private ClusterStatus
mapClusterStatus(org.apache.ignite.internal.cluster.management.ClusterState
clusterState) {
+ Set<String> metaStorageNodes = clusterState.metaStorageNodes();
+ long presentedMetaStorageNodes = topologyService.allMembers().stream()
+ .map(ClusterNode::name)
+ .filter(metaStorageNodes::contains)
+ .count();
+
+ if (presentedMetaStorageNodes <= metaStorageNodes.size() / 2) {
Review Comment:
```suggestion
if (presentMetaStorageNodes <= metaStorageNodes.size() / 2) {
```
##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/cluster/status/ItClusterStatusCommandInitializedTest.java:
##########
@@ -17,37 +17,79 @@
package org.apache.ignite.internal.cli.commands.cluster.status;
+import static java.util.function.Function.identity;
import static java.util.stream.Collectors.joining;
import static org.junit.jupiter.api.Assertions.assertAll;
import java.util.Arrays;
-import org.apache.ignite.Ignite;
+import java.util.Map;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
import org.apache.ignite.internal.cli.CliIntegrationTest;
+import org.jetbrains.annotations.Nullable;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
/**
* Tests for {@link ClusterStatusCommand} for the cluster that is initialized.
*/
class ItClusterStatusCommandInitializedTest extends CliIntegrationTest {
+ private Function<int[], String> mapper;
+
+ @Override
+ protected @Nullable int[] metastoreNodes() {
+ return new int[] { 0 };
+ }
+
+ @Override
+ protected @Nullable int[] cmgNodes() {
+ return new int[] { 1 };
+ }
+
@Test
@DisplayName("Should print status when valid cluster url is given but
cluster is initialized")
- void printStatus() {
- String cmgNodes = Arrays.stream(cmgMetastoreNodes())
- .mapToObj(CLUSTER::node)
- .map(Ignite::name)
+ void printStatus() throws InterruptedException {
+ String node0Url = NODE_URL;
+ String node1Url = "http://localhost:"; + CLUSTER.httpPort(1);
+
+ Map<Integer, String> nodeNames = IntStream.range(0, initialNodes())
+ .boxed()
+ .collect(Collectors.toMap(identity(), i ->
CLUSTER.node(i).name()));
+
+ mapper = nodes -> Arrays.stream(nodes)
+ .mapToObj(nodeNames::get)
.collect(joining(", ", "[", "]"));
- execute("cluster", "status", "--url", NODE_URL);
+ CLUSTER.stopNode(0);
+ execute("cluster", "status", "--url", node1Url);
+ assertOutput("cluster", 2, "Metastore majority lost", cmgNodes(),
metastoreNodes());
+
+ CLUSTER.startNode(0);
+ Thread.sleep(10000);
Review Comment:
Let's change this to `await().untilAsserted()`
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/call/cluster/status/ClusterStatusCall.java:
##########
@@ -30,13 +30,13 @@
import org.apache.ignite.rest.client.api.ClusterManagementApi;
import org.apache.ignite.rest.client.invoker.ApiException;
import org.apache.ignite.rest.client.model.ClusterNode;
-import org.apache.ignite.rest.client.model.ClusterState;
/**
* Call to get cluster status.
*/
@Singleton
-public class ClusterStatusCall implements Call<UrlCallInput, ClusterStatus> {
+public class ClusterStatusCall implements Call<UrlCallInput, ClusterState> {
+ private static final int READ_TIMEOUT = 50_000;
Review Comment:
Why this needs to be 50 seconds? As I understand this depends on the timeout
settings in the cluster state retrieval, can we at least point to the place
where this is configured in the CMGManager?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
