From 2c9b490c972a7d7bca698d03c2a212fcf22a7a63 Mon Sep 17 00:00:00 2001 From: Stefania Alborghetti Date: Fri, 10 Jul 2015 14:22:32 +0800 Subject: [PATCH 1/3] checkForEndpointCollision fails for legitimate collisions, CASSANDRA-9765 --- .../org/apache/cassandra/gms/Gossiper.java | 45 ++++++++++++++----- .../cassandra/service/StorageService.java | 2 +- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/java/org/apache/cassandra/gms/Gossiper.java b/src/java/org/apache/cassandra/gms/Gossiper.java index b77064d5e6..23eff82bf9 100644 --- a/src/java/org/apache/cassandra/gms/Gossiper.java +++ b/src/java/org/apache/cassandra/gms/Gossiper.java @@ -678,6 +678,16 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean } } + /** + * A fat client is a node that has not joined the ring, therefore acting as a coordinator only. + * It possesses no data. This method attempts to determine this property, except that for dead nodes + * we cannot tell. (??) We should also check that the node is not shutdown (and possibly other states) + * but due to fear of breaking things I added a new method to do this, isLiveFatClient(), see + * CASSANDRA-9765 for more information. + * + * @param endpoint - the endpoint we need to check + * @return true if it is a fat client + */ public boolean isFatClient(InetAddress endpoint) { EndpointState epState = endpointStateMap.get(endpoint); @@ -688,6 +698,11 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean return !isDeadState(epState) && !StorageService.instance.getTokenMetadata().isMember(endpoint); } + public boolean isLiveFatClient(InetAddress endpoint) + { + return isFatClient(endpoint) && !isShutdownState(endpointStateMap.get(endpoint)); + } + private void doStatusCheck() { if (logger.isTraceEnabled()) @@ -1008,12 +1023,9 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean public boolean isDeadState(EndpointState epState) { - if (epState.getApplicationState(ApplicationState.STATUS) == null) + String state = getApplicationState(epState); + if (state.isEmpty()) return false; - String value = epState.getApplicationState(ApplicationState.STATUS).value; - String[] pieces = value.split(VersionedValue.DELIMITER_STR, -1); - assert (pieces.length > 0); - String state = pieces[0]; for (String deadstate : DEAD_STATES) { if (state.equals(deadstate)) @@ -1024,12 +1036,9 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean public boolean isSilentShutdownState(EndpointState epState) { - if (epState.getApplicationState(ApplicationState.STATUS) == null) + String state = getApplicationState(epState); + if (state.isEmpty()) return false; - String value = epState.getApplicationState(ApplicationState.STATUS).value; - String[] pieces = value.split(VersionedValue.DELIMITER_STR, -1); - assert (pieces.length > 0); - String state = pieces[0]; for (String deadstate : SILENT_SHUTDOWN_STATES) { if (state.equals(deadstate)) @@ -1038,6 +1047,22 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean return false; } + public boolean isShutdownState(EndpointState epState) + { + return getApplicationState(epState).equals(VersionedValue.SHUTDOWN); + } + + private static String getApplicationState(EndpointState epState) + { + if (epState == null || epState.getApplicationState(ApplicationState.STATUS) == null) + return ""; + + String value = epState.getApplicationState(ApplicationState.STATUS).value; + String[] pieces = value.split(VersionedValue.DELIMITER_STR, -1); + assert (pieces.length > 0); + return pieces[0]; + } + void applyStateLocally(Map epStateMap) { for (Entry entry : epStateMap.entrySet()) diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index a256ce71c5..d70fff23b7 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -456,7 +456,7 @@ public class StorageService extends NotificationBroadcasterSupport implements IE MessagingService.instance().listen(FBUtilities.getLocalAddress()); Gossiper.instance.doShadowRound(); EndpointState epState = Gossiper.instance.getEndpointStateForEndpoint(FBUtilities.getBroadcastAddress()); - if (epState != null && !Gossiper.instance.isDeadState(epState) && !Gossiper.instance.isFatClient(FBUtilities.getBroadcastAddress())) + if (epState != null && !Gossiper.instance.isDeadState(epState) && !Gossiper.instance.isLiveFatClient(FBUtilities.getBroadcastAddress())) { throw new RuntimeException(String.format("A node with address %s already exists, cancelling join. " + "Use cassandra.replace_address if you want to replace this node.", From 54470a25f3c3c9ce6cb600c798ddcfe5e3962768 Mon Sep 17 00:00:00 2001 From: Stefania Alborghetti Date: Wed, 15 Jul 2015 16:30:22 +0800 Subject: [PATCH 2/3] checkForEndpointCollision fails for legitimate collisions, improved version after CR, CASSANDRA-9765 --- .../org/apache/cassandra/gms/Gossiper.java | 35 ++++++++++++------- .../cassandra/service/StorageService.java | 3 +- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/java/org/apache/cassandra/gms/Gossiper.java b/src/java/org/apache/cassandra/gms/Gossiper.java index 23eff82bf9..8eecc98d75 100644 --- a/src/java/org/apache/cassandra/gms/Gossiper.java +++ b/src/java/org/apache/cassandra/gms/Gossiper.java @@ -680,12 +680,8 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean /** * A fat client is a node that has not joined the ring, therefore acting as a coordinator only. - * It possesses no data. This method attempts to determine this property, except that for dead nodes - * we cannot tell. (??) We should also check that the node is not shutdown (and possibly other states) - * but due to fear of breaking things I added a new method to do this, isLiveFatClient(), see - * CASSANDRA-9765 for more information. * - * @param endpoint - the endpoint we need to check + * @param endpoint - the endpoint to check * @return true if it is a fat client */ public boolean isFatClient(InetAddress endpoint) @@ -698,9 +694,29 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean return !isDeadState(epState) && !StorageService.instance.getTokenMetadata().isMember(endpoint); } - public boolean isLiveFatClient(InetAddress endpoint) + /** + * Check if this endpoint can safely bootstrap into the cluster. + * + * @param endpoint - the endpoint to check + * @return true if the endpoint can join the cluster + */ + public boolean isSafeForBootstrap(InetAddress endpoint) { - return isFatClient(endpoint) && !isShutdownState(endpointStateMap.get(endpoint)); + EndpointState epState = endpointStateMap.get(endpoint); + String state = getApplicationState(epState); + logger.info("{} state : {}", endpoint, state); + + // if there's no previous state, or the node was previously removed from the cluster, we're good + if (epState == null || isDeadState(epState)) + return true; + + // these states are not allowed to join the cluster + List states = new ArrayList() {{ + add(""); // failed bootstrap but we did start gossiping + add(VersionedValue.STATUS_NORMAL); // node is legit in the cluster or was stopped kill -9 + //add(VersionedValue.STATUS_BOOTSTRAPPING); // failed bootstrap + add(VersionedValue.SHUTDOWN); }}; // node was shutdown + return !states.contains(state); } private void doStatusCheck() @@ -1047,11 +1063,6 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean return false; } - public boolean isShutdownState(EndpointState epState) - { - return getApplicationState(epState).equals(VersionedValue.SHUTDOWN); - } - private static String getApplicationState(EndpointState epState) { if (epState == null || epState.getApplicationState(ApplicationState.STATUS) == null) diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index d70fff23b7..745fe4c922 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -455,8 +455,7 @@ public class StorageService extends NotificationBroadcasterSupport implements IE if (!MessagingService.instance().isListening()) MessagingService.instance().listen(FBUtilities.getLocalAddress()); Gossiper.instance.doShadowRound(); - EndpointState epState = Gossiper.instance.getEndpointStateForEndpoint(FBUtilities.getBroadcastAddress()); - if (epState != null && !Gossiper.instance.isDeadState(epState) && !Gossiper.instance.isLiveFatClient(FBUtilities.getBroadcastAddress())) + if (!Gossiper.instance.isSafeForBootstrap(FBUtilities.getBroadcastAddress())) { throw new RuntimeException(String.format("A node with address %s already exists, cancelling join. " + "Use cassandra.replace_address if you want to replace this node.", From ba9a69ea21b6cf2e70408ba62522a4a58b695e3f Mon Sep 17 00:00:00 2001 From: Stefania Alborghetti Date: Thu, 16 Jul 2015 10:04:58 +0800 Subject: [PATCH 3/3] checkForEndpointCollision fails for legitimate collisions, finalized list of statuses and nits, CASSANDRA-9765 --- .../org/apache/cassandra/gms/Gossiper.java | 39 +++++++------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/src/java/org/apache/cassandra/gms/Gossiper.java b/src/java/org/apache/cassandra/gms/Gossiper.java index 8eecc98d75..8c36223986 100644 --- a/src/java/org/apache/cassandra/gms/Gossiper.java +++ b/src/java/org/apache/cassandra/gms/Gossiper.java @@ -703,20 +703,19 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean public boolean isSafeForBootstrap(InetAddress endpoint) { EndpointState epState = endpointStateMap.get(endpoint); - String state = getApplicationState(epState); - logger.info("{} state : {}", endpoint, state); // if there's no previous state, or the node was previously removed from the cluster, we're good if (epState == null || isDeadState(epState)) return true; - // these states are not allowed to join the cluster - List states = new ArrayList() {{ + String status = getGossipStatus(epState); + + // these states are not allowed to join the cluster as it would not be safe + final List unsafeStatuses = new ArrayList() {{ add(""); // failed bootstrap but we did start gossiping - add(VersionedValue.STATUS_NORMAL); // node is legit in the cluster or was stopped kill -9 - //add(VersionedValue.STATUS_BOOTSTRAPPING); // failed bootstrap + add(VersionedValue.STATUS_NORMAL); // node is legit in the cluster or it was stopped with kill -9 add(VersionedValue.SHUTDOWN); }}; // node was shutdown - return !states.contains(state); + return !unsafeStatuses.contains(status); } private void doStatusCheck() @@ -1039,31 +1038,23 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean public boolean isDeadState(EndpointState epState) { - String state = getApplicationState(epState); - if (state.isEmpty()) + String status = getGossipStatus(epState); + if (status.isEmpty()) return false; - for (String deadstate : DEAD_STATES) - { - if (state.equals(deadstate)) - return true; - } - return false; + + return DEAD_STATES.contains(status); } public boolean isSilentShutdownState(EndpointState epState) { - String state = getApplicationState(epState); - if (state.isEmpty()) + String status = getGossipStatus(epState); + if (status.isEmpty()) return false; - for (String deadstate : SILENT_SHUTDOWN_STATES) - { - if (state.equals(deadstate)) - return true; - } - return false; + + return SILENT_SHUTDOWN_STATES.contains(status); } - private static String getApplicationState(EndpointState epState) + private static String getGossipStatus(EndpointState epState) { if (epState == null || epState.getApplicationState(ApplicationState.STATUS) == null) return "";