From 1d13bafea425f0cfc3b9b5f8cb23aed220652bda Mon Sep 17 00:00:00 2001 From: Alexander Dejanovski Date: Sat, 21 Oct 2023 13:28:42 +0200 Subject: [PATCH 1/2] Filter out Stargate/coordinator nodes from calls to get live nodes or list tokens. The http management implementation uses endpoint states for listing nodes and tokens, unlike JMX, which also contains Stargate nodes. This breaks some calls which expect the Stargate nodes to behave like standard Cassandra nodes. This commit filters out the endpoint state entries that don't have tokens to make sure Stargate nodes aren't accounted for when they shouldn't be. --- .../http/HttpCassandraManagementProxy.java | 24 +++++++++--- .../http/HttpManagementConnectionFactory.java | 2 +- .../HttpCassandraManagementProxyTest.java | 37 ++++++++++++++++++- 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/server/src/main/java/io/cassandrareaper/management/http/HttpCassandraManagementProxy.java b/src/server/src/main/java/io/cassandrareaper/management/http/HttpCassandraManagementProxy.java index 5b55bfcb5..e12500898 100644 --- a/src/server/src/main/java/io/cassandrareaper/management/http/HttpCassandraManagementProxy.java +++ b/src/server/src/main/java/io/cassandrareaper/management/http/HttpCassandraManagementProxy.java @@ -154,8 +154,11 @@ public List getTokens() { EndpointStates endpointStates = apiClient.getEndpointStates(); List tokenList = new ArrayList<>(); endpointStates.getEntity().forEach((Map states) -> { - for (String token : states.get("TOKENS").split(",")) { - tokenList.add(new BigInteger(token)); + // Stargate nodes are part of the endpoint states but do not have tokens + if (!isCoordinatorNode(states)) { + for (String token : states.get("TOKENS").split(",")) { + tokenList.add(new BigInteger(token)); + } } }); // sort the list @@ -405,7 +408,9 @@ public List getLiveNodes() throws ReaperException { List liveNodes = new ArrayList<>(); EndpointStates endpoints = apiClient.getEndpointStates(); for (Map states : endpoints.getEntity()) { - if (states.containsKey("IS_ALIVE") && Boolean.parseBoolean(states.get("IS_ALIVE"))) { + if (states.containsKey("IS_ALIVE") + && Boolean.parseBoolean(states.get("IS_ALIVE")) + && !isCoordinatorNode(states)) { liveNodes.add(states.get("ENDPOINT_IP")); } } @@ -499,8 +504,11 @@ public Map getTokenToEndpointMap() { Map tokenMap = new HashMap<>(); for (Map states : epStates.getEntity()) { String ip = states.get("ENDPOINT_IP"); - for (String token : states.get("TOKENS").split(",")) { - tokenMap.put(token, ip); + // Stargate nodes are part of the endpoint states but do not have tokens + if (!isCoordinatorNode(states)) { + for (String token : states.get("TOKENS").split(",")) { + tokenMap.put(token, ip); + } } } return tokenMap; @@ -637,4 +645,10 @@ Runnable notificationsTracker() { } }; } + + // Coordinator nodes such as Stargate instances do not have tokens + @VisibleForTesting + static boolean isCoordinatorNode(Map endpointState) { + return endpointState.getOrDefault("TOKENS", "null").equals("null"); + } } \ No newline at end of file diff --git a/src/server/src/main/java/io/cassandrareaper/management/http/HttpManagementConnectionFactory.java b/src/server/src/main/java/io/cassandrareaper/management/http/HttpManagementConnectionFactory.java index 1356778ea..b88216eac 100644 --- a/src/server/src/main/java/io/cassandrareaper/management/http/HttpManagementConnectionFactory.java +++ b/src/server/src/main/java/io/cassandrareaper/management/http/HttpManagementConnectionFactory.java @@ -91,7 +91,7 @@ public ICassandraManagementProxy connectAny(Collection nodes) throws Reape return cassandraManagementProxy; } catch (ReaperException | RuntimeException | UnknownHostException e) { getHostConnectionCounters().decrementSuccessfulConnections(node.getHostname()); - LOG.info("Unreachable host: ", e); + LOG.info("Unreachable host: {}", node.getHostname(), e); } catch (InterruptedException expected) { LOG.trace("Expected exception", expected); } diff --git a/src/server/src/test/java/io/cassandrareaper/management/http/HttpCassandraManagementProxyTest.java b/src/server/src/test/java/io/cassandrareaper/management/http/HttpCassandraManagementProxyTest.java index ffd2a28d2..a0121396a 100644 --- a/src/server/src/test/java/io/cassandrareaper/management/http/HttpCassandraManagementProxyTest.java +++ b/src/server/src/test/java/io/cassandrareaper/management/http/HttpCassandraManagementProxyTest.java @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -64,6 +65,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -487,7 +489,6 @@ public static HttpCassandraManagementProxy mockProxy(DefaultApi mockClient) { @Test public void testGetTokens() throws Exception { - DefaultApi mockClient = Mockito.mock(DefaultApi.class); List> mockEntity = new ArrayList<>(); mockEntity.add(ImmutableMap.of( "TOKENS", "1,2,3,4", @@ -499,7 +500,17 @@ public void testGetTokens() throws Exception { "IS_LOCAL", "false" ) ); + mockEntity.add(ImmutableMap.of( + "TOKENS", "null", + "IS_LOCAL", "false" + ) + ); + mockEntity.add(ImmutableMap.of( + "IS_LOCAL", "false" + ) + ); EndpointStates mockEndpointStates = new EndpointStates().entity(mockEntity); + DefaultApi mockClient = Mockito.mock(DefaultApi.class); when(mockClient.getEndpointStates()).thenReturn(mockEndpointStates); mockProxy(mockClient); assertThat(mockProxy(mockClient).getTokens()).containsOnly( @@ -763,4 +774,28 @@ public void testGetPendingCompactionsFail() throws ReaperException { metricsProxy); proxy.getPendingCompactions(); } + + @Test + //Test when endpoint state contains TOKENS and it is not null + public void testIsCoordinatorNode_WhenTokensNonNull() { + HashMap endpointState = new HashMap(); + endpointState.put("TOKENS", "12345"); + assertFalse(HttpCassandraManagementProxy.isCoordinatorNode(endpointState)); + } + + @Test + //Test when endpoint state contains TOKENS and it is null + public void testIsCoordinatorNode_WhenTokensNull() { + HashMap endpointState = new HashMap(); + endpointState.put("TOKENS", "null"); + assertTrue(HttpCassandraManagementProxy.isCoordinatorNode(endpointState)); + } + + @Test + //Test when endpoint state does not contain TOKENS + public void testIsCoordinatorNode_NoTokens() { + HashMap endpointState = new HashMap(); + endpointState.put("OTHER", "value"); + assertTrue(HttpCassandraManagementProxy.isCoordinatorNode(endpointState)); + } } From 0c33959a3c83397af17749fdb754f4fa57e09d61 Mon Sep 17 00:00:00 2001 From: Alexander Dejanovski Date: Mon, 23 Oct 2023 15:54:34 +0200 Subject: [PATCH 2/2] Address review comment --- .../management/http/HttpCassandraManagementProxy.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server/src/main/java/io/cassandrareaper/management/http/HttpCassandraManagementProxy.java b/src/server/src/main/java/io/cassandrareaper/management/http/HttpCassandraManagementProxy.java index e12500898..b21233e0d 100644 --- a/src/server/src/main/java/io/cassandrareaper/management/http/HttpCassandraManagementProxy.java +++ b/src/server/src/main/java/io/cassandrareaper/management/http/HttpCassandraManagementProxy.java @@ -408,9 +408,9 @@ public List getLiveNodes() throws ReaperException { List liveNodes = new ArrayList<>(); EndpointStates endpoints = apiClient.getEndpointStates(); for (Map states : endpoints.getEntity()) { - if (states.containsKey("IS_ALIVE") - && Boolean.parseBoolean(states.get("IS_ALIVE")) - && !isCoordinatorNode(states)) { + if (!isCoordinatorNode(states) + && states.containsKey("IS_ALIVE") + && Boolean.parseBoolean(states.get("IS_ALIVE"))) { liveNodes.add(states.get("ENDPOINT_IP")); } }