diff --git a/CHANGES.txt b/CHANGES.txt index a8a5ddeaeb..c5f2a1b92c 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -5,6 +5,7 @@ * Allow empty keystore_password in encryption_options (CASSANDRA-18778) * Skip ColumnFamilyStore#topPartitions initialization when client or tool mode (CASSANDRA-18697) Merged from 4.0: + * Gossip NPE due to shutdown event corrupting empty statuses (CASSANDRA-18913) * Fix closing iterator in SecondaryIndexBuilder (CASSANDRA-18361) * Update hdrhistogram to 2.1.12 (CASSANDRA-18893) * Improve performance of compactions when table does not have an index (CASSANDRA-18773) diff --git a/src/java/org/apache/cassandra/gms/EndpointState.java b/src/java/org/apache/cassandra/gms/EndpointState.java index c60a4793bd..198946fa51 100644 --- a/src/java/org/apache/cassandra/gms/EndpointState.java +++ b/src/java/org/apache/cassandra/gms/EndpointState.java @@ -196,6 +196,11 @@ public class EndpointState isAlive = false; } + public boolean isStateEmpty() + { + return applicationState.get().isEmpty(); + } + /** * @return true if {@link HeartBeatState#isEmpty()} is true and no STATUS application state exists */ diff --git a/src/java/org/apache/cassandra/gms/Gossiper.java b/src/java/org/apache/cassandra/gms/Gossiper.java index 4a32662dcf..0d5db5f81c 100644 --- a/src/java/org/apache/cassandra/gms/Gossiper.java +++ b/src/java/org/apache/cassandra/gms/Gossiper.java @@ -590,7 +590,7 @@ public class Gossiper implements IFailureDetectionEventListener, GossiperMBean { checkProperThreadForStateMutation(); EndpointState epState = endpointStateMap.get(endpoint); - if (epState == null) + if (epState == null || epState.isStateEmpty()) return; VersionedValue shutdown = StorageService.instance.valueFactory.shutdown(true); epState.addApplicationState(ApplicationState.STATUS_WITH_PORT, shutdown); diff --git a/test/distributed/org/apache/cassandra/distributed/test/gossip/GossipShutdownTest.java b/test/distributed/org/apache/cassandra/distributed/test/gossip/GossipShutdownTest.java new file mode 100644 index 0000000000..59ac4feeeb --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/gossip/GossipShutdownTest.java @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.distributed.test.gossip; + +import java.io.IOException; +import java.util.concurrent.CountDownLatch; + +import org.junit.Test; + +import org.apache.cassandra.distributed.Cluster; +import org.apache.cassandra.distributed.api.Feature; +import org.apache.cassandra.distributed.api.IInvokableInstance; +import org.apache.cassandra.distributed.api.TokenSupplier; +import org.apache.cassandra.distributed.shared.ClusterUtils; +import org.apache.cassandra.distributed.test.TestBaseImpl; +import org.apache.cassandra.net.Verb; + +public class GossipShutdownTest extends TestBaseImpl +{ + @Test + public void emptyStateAndShutdownEvent() throws IOException + { + try (Cluster cluster = builder() + .withNodes(2) + .withConfig(c -> c.with(Feature.values())) + .start()) + { + IInvokableInstance n1 = cluster.get(1); + IInvokableInstance n2 = cluster.get(2); + ClusterUtils.awaitRingJoin(n1, n2); + ClusterUtils.awaitRingJoin(n2, n1); + + ClusterUtils.stopUnchecked(n1); + + // make sure we don't see gossip events, but do allow gossip shutdown + cluster.filters().verbs(Verb.GOSSIP_DIGEST_ACK.id, + Verb.GOSSIP_DIGEST_ACK.id, + Verb.GOSSIP_DIGEST_ACK2.id) + .to(1) + .drop(); + + n1.startup(); + + n2.nodetoolResult("disablegossip").asserts().success(); + } + } + + @Test + public void emptyStateAndHostJoin() throws IOException, InterruptedException + { + try (Cluster cluster = builder() + .withNodes(2) + .withTokenSupplier(TokenSupplier.evenlyDistributedTokens(3)) + .withConfig(c -> c.with(Feature.values())) + .start()) + { + IInvokableInstance n1 = cluster.get(1); + IInvokableInstance n2 = cluster.get(2); + + ClusterUtils.awaitRingJoin(n1, n2); + ClusterUtils.awaitRingJoin(n2, n1); + + ClusterUtils.stopUnchecked(n1); + + // make sure we don't see gossip events, but do allow gossip shutdown + cluster.filters().verbs(Verb.GOSSIP_DIGEST_ACK.id, + Verb.GOSSIP_DIGEST_ACK.id, + Verb.GOSSIP_DIGEST_ACK2.id) + .to(1) + .drop(); + + n1.startup(); + + CountDownLatch latch = new CountDownLatch(1); + cluster.filters().verbs(Verb.GOSSIP_SHUTDOWN.id).messagesMatching((from, to, iMessage) -> { + latch.countDown(); + return false; + }).drop(); + + n2.nodetoolResult("disablegossip").asserts().success(); + + latch.await(); + cluster.filters().reset(); + + IInvokableInstance n3 = ClusterUtils.addInstance(cluster, n1.config(), c -> {}); + + n3.startup(); + } + } +}