From b030fc134d9651c83a02bd1954e7426393915b84 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Thu, 29 May 2025 12:32:37 +0100 Subject: [PATCH 01/27] [CASSANDRA-20476] Add dtest for CMS rediscovery --- .../distributed/shared/ClusterUtils.java | 40 ++- .../test/log/DiscoverNewCMSTest.java | 228 ++++++++++++++++++ 2 files changed, 266 insertions(+), 2 deletions(-) create mode 100644 test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java diff --git a/test/distributed/org/apache/cassandra/distributed/shared/ClusterUtils.java b/test/distributed/org/apache/cassandra/distributed/shared/ClusterUtils.java index 006379ae3bfc..1d54bad404d9 100644 --- a/test/distributed/org/apache/cassandra/distributed/shared/ClusterUtils.java +++ b/test/distributed/org/apache/cassandra/distributed/shared/ClusterUtils.java @@ -24,7 +24,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -78,6 +80,7 @@ import org.apache.cassandra.gms.VersionedValue; import org.apache.cassandra.io.util.File; import org.apache.cassandra.locator.InetAddressAndPort; +import org.apache.cassandra.locator.SimpleSeedProvider; import org.apache.cassandra.net.Message; import org.apache.cassandra.net.MessagingService; import org.apache.cassandra.net.RequestCallback; @@ -107,6 +110,7 @@ import static org.apache.cassandra.config.CassandraRelevantProperties.BROADCAST_INTERVAL_MS; import static org.apache.cassandra.config.CassandraRelevantProperties.REPLACE_ADDRESS_FIRST_BOOT; import static org.apache.cassandra.config.CassandraRelevantProperties.RING_DELAY; +import static org.apache.cassandra.distributed.impl.TestEndpointCache.fromCassandraInetAddressAndPort; import static org.apache.cassandra.distributed.impl.TestEndpointCache.toCassandraInetAddressAndPort; import static org.assertj.core.api.Assertions.assertThat; @@ -834,6 +838,21 @@ public static Set getCMSMembers(IInvokableInstance inst) .collect(Collectors.toSet())); } + public static Set getCMSMemberIds(IInvokableInstance inst) + { + Set idsAsInts = inst.callOnInstance(() -> ClusterMetadata.current() + .fullCMSMemberIds() + .stream() + .map(NodeId::id) + .collect(Collectors.toSet())); + return idsAsInts.stream().map(NodeId::new).collect(Collectors.toSet()); + } + + public static Set getCMSMemberAddresses(IInvokableInstance inst) + { + return inst.callOnInstance(() -> new HashSet<>(ClusterMetadata.current().fullCMSMembers())); + } + public static boolean decommission(IInvokableInstance leaving) { return leaving.callOnInstance(() -> { @@ -871,6 +890,12 @@ public static int getNodeId(IInvokableInstance target, IInvokableInstance execut }); } + public static InetSocketAddress getEndpoint(IInvokableInstance target, NodeId id) + { + String idString = Integer.toString(id.id()); + return target.callOnInstance(() -> fromCassandraInetAddressAndPort(ClusterMetadata.current().directory.endpoint(NodeId.fromString(idString)))); + } + public static boolean cancelInProgressSequences(IInvokableInstance executor) { return cancelInProgressSequences(getNodeId(executor), executor); @@ -1443,6 +1468,15 @@ public static String getPartitionerName(IInstance instance) return (String) instance.config().get("partitioner"); } + + public static void updateSeed(IInstance instance, String...address) + { + IInstanceConfig conf = instance.config(); + conf.set("seed_provider", + new IInstanceConfig.ParameterizedClass(SimpleSeedProvider.class.getName(), + Collections.singletonMap("seeds", String.join(",", address)))); + } + /** * Changes the instance's address to the new address. This method should only be called while the instance is * down, else has undefined behavior. @@ -1478,12 +1512,14 @@ private static void updateAddress(IInstanceConfig conf, String address) // are a risk if (!conf.broadcastAddress().equals(previous)) { - conf.networkTopology().put(conf.broadcastAddress(), NetworkTopology.dcAndRack(conf.localDatacenter(), conf.localRack())); + NetworkTopology topology = conf.networkTopology(); + NetworkTopology.DcAndRack location = NetworkTopology.dcAndRack(topology.localDC(previous), topology.localRack(previous)); + topology.put(conf.broadcastAddress(), location); try { Field field = NetworkTopology.class.getDeclaredField("map"); field.setAccessible(true); - Map map = (Map) field.get(conf.networkTopology()); + Map map = (Map) field.get(topology); map.remove(previous); } catch (NoSuchFieldException | IllegalAccessException e) diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java b/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java new file mode 100644 index 000000000000..f73465aa476a --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java @@ -0,0 +1,228 @@ +/* + * 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.log; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import com.google.common.base.Throwables; + +import org.awaitility.Awaitility; +import org.junit.Before; +import org.junit.Test; + +import org.apache.cassandra.config.CassandraRelevantProperties; +import org.apache.cassandra.cql3.QueryProcessor; +import org.apache.cassandra.cql3.UntypedResultSet; +import org.apache.cassandra.distributed.Cluster; +import org.apache.cassandra.distributed.api.IInvokableInstance; +import org.apache.cassandra.distributed.shared.ClusterUtils; +import org.apache.cassandra.distributed.test.TestBaseImpl; +import org.apache.cassandra.tcm.Epoch; +import org.apache.cassandra.tcm.membership.NodeId; +import org.apache.cassandra.utils.FBUtilities; +import org.apache.cassandra.utils.Pair; + +import static org.apache.cassandra.distributed.api.Feature.GOSSIP; +import static org.apache.cassandra.distributed.api.Feature.NETWORK; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class DiscoverNewCMSTest extends TestBaseImpl +{ + + @Before + public void disableAccord() + { + CassandraRelevantProperties.DTEST_ACCORD_ENABLED.setBoolean(false); + } + + @Test + public void singleNodeCMSAddressChangeTest() throws IOException, ExecutionException, InterruptedException + { + try (Cluster cluster = builder().withNodes(3) + .withConfig(config -> config.with(NETWORK, GOSSIP)) + .createWithoutStarting()) + { + test(cluster, 1); + } + } + + @Test + public void multiNodeCMSOnlyClusterAddressChangeTest() throws IOException, ExecutionException, InterruptedException + { + + try (Cluster cluster = builder().withNodes(3) + .withConfig(config -> config.with(NETWORK, GOSSIP)) + .createWithoutStarting()) + { + test(cluster, 3); + } + } + + @Test + public void multiNodeCMSAllAddressesChangeTest() throws IOException, ExecutionException, InterruptedException + { + try (Cluster cluster = builder().withNodes(6) + .withConfig(config -> config.with(NETWORK, GOSSIP)) + .createWithoutStarting()) + { + test(cluster, 3); + } + } + + private void test(Cluster cluster, int cmsSize) throws IOException, ExecutionException, InterruptedException + { + ExecutorService executor = Executors.newFixedThreadPool(cluster.size()); + cluster.setUncaughtExceptionsFilter((node, t) -> { + Throwable rootCause = Throwables.getRootCause(t); + // Some fetchCMSLog operations might temporarily fail and be retried during address changes + return rootCause.getMessage() != null + && rootCause.getMessage().startsWith("Cannot achieve consistency level SERIAL"); + + }); + cluster.startup(); + init(cluster); + IInvokableInstance n1 = cluster.get(1); + if (cmsSize > 1) + n1.nodetoolResult("cms", "reconfigure", "" + cmsSize).asserts().success(); + ClusterUtils.waitForCMSToQuiesce(cluster, n1); + + // Set up the expectations for what address changes are going to happen + Map> addressMapping = new HashMap<>(cluster.size()); + for (IInvokableInstance inst : cluster) + { + InetSocketAddress starting = inst.config().broadcastAddress(); + InetSocketAddress expected = new InetSocketAddress(bumpAddress(starting.getAddress()), starting.getPort()); + NodeId id = ClusterUtils.getNodeId(inst); + addressMapping.put(starting, Pair.create(id, expected)); + } + + // Check the CMS membership at the start of the test & predict what it should be at the end + Set startingCMS = new HashSet<>(cmsSize); + Set expectedCMS = new HashSet<>(cmsSize); + for (InetSocketAddress s : ClusterUtils.getCMSMemberAddresses(n1)) + { + startingCMS.add(s); + expectedCMS.add(addressMapping.get(s).right); + } + Set cmsNodes = ClusterUtils.getCMSMemberIds(n1); + + // Shut down all nodes, modify each one's broadcast address and reconfigure seeds as these + // will be used to rediscover peers. Seed config does not need to be uniform across the cluster + // but there must be enough intersection to enable the CMS members to rediscover each other + for (int i = 1; i <= cluster.size(); i++) + { + IInvokableInstance inst = cluster.get(i); + inst.shutdown().get(); + InetAddress newBroadcastAddress = addressMapping.get(inst.config().broadcastAddress()).right.getAddress(); + byte[] bytes = newBroadcastAddress.getAddress(); + ClusterUtils.updateAddress(inst, addrString(bytes)); + String seed1 = addrString(bytes[0], bytes[1], bytes[2], (byte) i); + String seed2 = addrString(bytes[0], bytes[1], bytes[2], (byte) ((i < cluster.size()) ? i + 1 : 1)); + ClusterUtils.updateSeed(inst, seed1, seed2); + } + + // Start everything up and wait for state to cluster state to quiesce + List> startups = new ArrayList<>(cluster.size()); + for (IInvokableInstance inst : cluster) + { + Future f = executor.submit(() -> { + inst.startup(); + return true; + }); + startups.add(f); + } + + FBUtilities.waitOnFutures(startups, 60, TimeUnit.SECONDS); + ClusterUtils.waitForCMSToQuiesce(cluster, n1); + + // wait until each node's STARTUP transformation has been enacted by all nodes + Awaitility.waitAtMost(30, TimeUnit.SECONDS).until(() -> allAddressChangesEnacted(cluster)); + Epoch afterAllAddressChanges = getEpochAfterAllAddressChanges(cluster); + ClusterUtils.waitForCMSToQuiesce(cluster, afterAllAddressChanges, true); + + // Assert that: + // * The membership of the CMS (i.e. which node ids) remains the same + // * The set of CMS addresses matches the prediction made at the start + // * Every node has successfully changed its address. The previous check + // is a logical consequence of this, but it doesn't hurt to verify both + for (IInvokableInstance inst : cluster) + { + assertEquals(cmsNodes, ClusterUtils.getCMSMemberIds(inst)); + Set finalCMS = ClusterUtils.getCMSMemberAddresses(inst); + assertEquals(startingCMS.size(), finalCMS.size()); + assertEquals(expectedCMS.size(), finalCMS.size()); + assertTrue(expectedCMS.containsAll(finalCMS)); + for (Pair peer : addressMapping.values()) + { + InetSocketAddress fromInst = ClusterUtils.getEndpoint(inst, peer.left); + assertEquals("Check failed on instance " + inst.config().num(), peer.right, fromInst); + } + } + } + + private boolean allAddressChangesEnacted(Cluster cluster) + { + return getEpochAfterAllAddressChanges(cluster).isAfter(Epoch.FIRST); + } + + private Epoch getEpochAfterAllAddressChanges(Cluster cluster) + { + int nodes = cluster.size(); + long epochAfterAllStartups = cluster.get(1).callOnInstance(() -> { + UntypedResultSet rs = QueryProcessor.executeInternal("SELECT epoch, kind from system_views.cluster_metadata_log where kind = 'STARTUP'"); + if (rs == null || rs.isEmpty() || rs.size() < nodes) + return -1; + + long epoch = rs.stream().mapToLong(r -> r.getLong("epoch")).max().orElse(-1); + return epoch; + }).longValue(); + return Epoch.create(epochAfterAllStartups); + } + + private InetAddress bumpAddress(InetAddress address) throws UnknownHostException + { + // ipv4 addresses for this test + assert address.getAddress().length == 4; + byte[] bytes = address.getAddress(); + bytes[2]++; + return InetAddress.getByAddress(bytes); + } + + private String addrString(byte...b) throws UnknownHostException + { + assert b.length == 4; + return InetAddress.getByAddress(new byte[] { b[0], b[1], b[2], b[3] }).getHostAddress(); + } + +} From e67e83f92d8ef57a863a42a2a9aedd74484c860b Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 23 Jun 2025 16:12:29 +0100 Subject: [PATCH 02/27] [CASSANDRA-20476] Introduce CMSLookup --- .../org/apache/cassandra/tcm/CMSLookup.java | 185 ++++++++++++++++++ .../apache/cassandra/tcm/CMSMembership.java | 7 +- .../apache/cassandra/tcm/ClusterMetadata.java | 88 ++++++++- .../cassandra/tcm/ClusterMetadataService.java | 3 +- src/java/org/apache/cassandra/tcm/Commit.java | 13 +- .../cassandra/tcm/membership/Directory.java | 2 +- .../tcm/membership/EndpointLookup.java | 26 +++ 7 files changed, 304 insertions(+), 20 deletions(-) create mode 100644 src/java/org/apache/cassandra/tcm/CMSLookup.java create mode 100644 src/java/org/apache/cassandra/tcm/membership/EndpointLookup.java diff --git a/src/java/org/apache/cassandra/tcm/CMSLookup.java b/src/java/org/apache/cassandra/tcm/CMSLookup.java new file mode 100644 index 000000000000..6713b7f75a55 --- /dev/null +++ b/src/java/org/apache/cassandra/tcm/CMSLookup.java @@ -0,0 +1,185 @@ +/* + * 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.tcm; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +import com.google.common.collect.BiMap; +import com.google.common.collect.HashBiMap; +import com.google.common.collect.Maps; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.locator.InetAddressAndPort; +import org.apache.cassandra.tcm.listeners.ChangeListener; +import org.apache.cassandra.tcm.membership.EndpointLookup; +import org.apache.cassandra.tcm.membership.NodeId; +import org.apache.cassandra.utils.Pair; + +public class CMSLookup +{ + private static final Logger logger = LoggerFactory.getLogger(CMSLookup.class); + + public enum State { PRE_INIT, ACTIVE, RETIRED }; + + public final static CMSLookup NO_OP = new CMSLookup(State.PRE_INIT, Epoch.EMPTY, new HashMap<>()); + public static InitialBuilder builder(ClusterMetadata metadata) + { + return new InitialBuilder(metadata); + } + + private final Map> overrides; + private final BiMap addressMap; + private final Epoch lastModified; + private final State state; + + private CMSLookup(State state, Epoch epoch, Map> overrides) + { + this.state = state; + this.lastModified = epoch; + this.addressMap = HashBiMap.create(overrides.size()); + this.overrides = Maps.newHashMapWithExpectedSize(overrides.size()); + for (Map.Entry> e : overrides.entrySet()) + { + this.overrides.put(e.getKey(), e.getValue()); + this.addressMap.put(e.getValue().left, e.getValue().right); + } + } + + public boolean isUninitialized() + { + return state == State.PRE_INIT; + } + + public boolean isActive() + { + return state == State.ACTIVE; + } + + public InetAddressAndPort getAddressOverride(NodeId id) + { + Pair override = overrides.get(id); + return override != null ? override.right : null; + } + + public EndpointLookup asNodeLookup(EndpointLookup lookup) + { + return new EndpointLookup() + { + @Override + public InetAddressAndPort endpoint(NodeId id) + { + if (overrides.containsKey(id)) + return overrides.get(id).right; + return lookup.endpoint(id); + } + }; + } + + public CMSLookup rebuild(ClusterMetadata prev, ClusterMetadata next, boolean fromSnapshot) + { + logger.debug("Rebuilding CMS lookup {} with metadata from epoch {}", this, next.epoch.getEpoch()); + + // All address changes have been enacted, nothing to do + if (state == State.RETIRED) + return this; + + if (!next.epoch.isEqualOrBefore(Epoch.FIRST) + && !fromSnapshot + && next.directory.lastModified().equals(prev.directory.lastModified())) + return this; + + // Filters from the override list those which are no longer necessary as a transformation has now + // replaced the old address with the new one for that node id + Predicate>> overrideNowEnacted = entry -> { + NodeId nodeId = entry.getKey(); + if (!Objects.equals(prev.directory.getNodeAddresses(nodeId), next.directory.getNodeAddresses(nodeId))) + { + Pair override = overrides.get(nodeId); + if (override != null) + { + // If there was an override for this nodeId && the address being overriden matches the prev + // directory entry, filter out the override from the map which will be used to build the new version + // (i.e. return false from Predicate::test). This indicates the override is no longer required. + return !override.left.equals(prev.directory.endpoint(nodeId)); + } + } + return true; + }; + + logger.debug("Current endpoint overrides: {}", overrides); + Map> nextOverrides + = overrides.entrySet() + .stream() + .filter(overrideNowEnacted) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + logger.debug("Proposed endpoint overrides: {}", nextOverrides); + State state = nextOverrides.isEmpty() ? State.RETIRED : State.ACTIVE; + return new CMSLookup(state, next.epoch, nextOverrides); + } + + @Override + public String toString() + { + return "CMSLookup{" + + "state=" + state + + ", epoch=" + lastModified + + ", overrides=" + overrides + + ", addressMap=" + addressMap + + '}'; + } + + public static class InitialBuilder + { + private final Epoch epoch; + private final Map> overrides; + + private InitialBuilder(ClusterMetadata metadata) + { + this.epoch = metadata.epoch; + this.overrides = new HashMap<>(); + } + + public InitialBuilder withOverride(NodeId id, InetAddressAndPort originalAddress, InetAddressAndPort newAddress) + { + overrides.put(id, Pair.create(originalAddress, newAddress)); + return this; + } + + public CMSLookup build() + { + return new CMSLookup(State.ACTIVE, epoch, overrides); + } + } + + public static class LogListener implements ChangeListener + { + @Override + public void notifyPreCommit(ClusterMetadata prev, ClusterMetadata next, boolean fromSnapshot) + { + logger.debug("Reevaluating CMSLookup from {} at epoch {}", prev.epoch, next.epoch); + next.refreshCMSLookup(prev, fromSnapshot); + } + } +} diff --git a/src/java/org/apache/cassandra/tcm/CMSMembership.java b/src/java/org/apache/cassandra/tcm/CMSMembership.java index 67c30dc86085..9e73ea9e61ac 100644 --- a/src/java/org/apache/cassandra/tcm/CMSMembership.java +++ b/src/java/org/apache/cassandra/tcm/CMSMembership.java @@ -28,6 +28,7 @@ import org.apache.cassandra.locator.MetaStrategy; import org.apache.cassandra.locator.Replica; import org.apache.cassandra.tcm.membership.Directory; +import org.apache.cassandra.tcm.membership.EndpointLookup; import org.apache.cassandra.tcm.membership.NodeId; import org.apache.cassandra.tcm.ownership.DataPlacement; import org.apache.cassandra.tcm.ownership.VersionedEndpoints; @@ -80,18 +81,18 @@ public static CMSMembership reconstruct(DataPlacement placement, Directory direc return new CMSMembership(lm, full, joining); } - public DataPlacement toPlacement(Directory directory) + public DataPlacement toPlacement(EndpointLookup lookup) { DataPlacement.Builder builder = DataPlacement.builder(); for (NodeId id : fullMembers) { - Replica replica = MetaStrategy.replica(directory.endpoint(id)); + Replica replica = MetaStrategy.replica(lookup.endpoint(id)); builder.withReadReplica(lastModified, replica); builder.withWriteReplica(lastModified, replica); } for(NodeId id : joiningMembers) { - builder.withWriteReplica(lastModified, MetaStrategy.replica(directory.endpoint(id))); + builder.withWriteReplica(lastModified, MetaStrategy.replica(lookup.endpoint(id))); } return builder.build(); } diff --git a/src/java/org/apache/cassandra/tcm/ClusterMetadata.java b/src/java/org/apache/cassandra/tcm/ClusterMetadata.java index 0224ca5bfbb2..3b93bb63c7dd 100644 --- a/src/java/org/apache/cassandra/tcm/ClusterMetadata.java +++ b/src/java/org/apache/cassandra/tcm/ClusterMetadata.java @@ -74,6 +74,7 @@ import org.apache.cassandra.tcm.extensions.ExtensionKey; import org.apache.cassandra.tcm.extensions.ExtensionValue; import org.apache.cassandra.tcm.membership.Directory; +import org.apache.cassandra.tcm.membership.EndpointLookup; import org.apache.cassandra.tcm.membership.Location; import org.apache.cassandra.tcm.membership.NodeAddresses; import org.apache.cassandra.tcm.membership.NodeId; @@ -121,6 +122,12 @@ public class ClusterMetadata // This isn't serialized as part of ClusterMetadata it's really just a view over the Directory. public final Locator locator; + // This isn't serialized, it's a helper to apply transient (or not yet committed) changes to CMS member addresses. + // It is initialized with any address changes detected in Startup::initializeCMSLookup when the node boots. It is + // subsequently updated exclusively by CMSLookup.LogListener when enacting a transformation which modifies the + // addresses of CMS member(s), making those changes permanent/durable. + public volatile CMSLookup cmsLookup = CMSLookup.NO_OP; + // These fields are lazy but only for the test purposes, since their computation requires initialization of the log ks private EndpointsForRange fullCMSReplicas; private Set fullCMSEndpoints; @@ -189,7 +196,6 @@ public ClusterMetadata(Epoch epoch, cmsMembership); } - private ClusterMetadata(int metadataIdentifier, Epoch epoch, IPartitioner partitioner, @@ -224,8 +230,8 @@ private ClusterMetadata(int metadataIdentifier, this.locator = Locator.usingDirectory(directory); this.accordStaleReplicas = accordStaleReplicas; this.cmsMembership = cmsMembership; - this.cmsDataPlacement = calculateCMSPlacement(placements, cmsMembership); - + // Build CMS placement using no-op CMS lookup, i.e. using only committed node addresses + this.cmsDataPlacement = calculateCMSPlacement(placements, cmsMembership, CMSLookup.NO_OP); InetAddressAndPort broadcastAddress = FBUtilities.getBroadcastAddressAndPort(); this.localNodeId = directory.allAddresses().contains(broadcastAddress) ? directory.peerId(broadcastAddress) @@ -239,7 +245,25 @@ public Set fullCMSMemberIds() public boolean isCMSMember() { - if (epoch.isEqualOrBefore(Epoch.FIRST)) + // There are two special cases to consider here: + // * Initialization of the CMS for the first time by its first member + // During first initialization, placements are hardcoded to the local address after the PRE_INITIALIZE_CMS is + // committed so that the subsequent INITIALIZE_CMS can be. + // + // * Some changes to the addresses of the CMS members have been changed and whilst this is known (i.e. has been + // discovered) it has not yet been committed. + // When address changes are in flight, at startup the *current* broadcast address for the local node will not + // yet be associated with its NodeId in the directory, causing the localNodeId to be set to + // NodeId.UNREGISTERED. Updating the address requires a Startup transformation to be committed, but if the + // node itself is a CMS member there is a chicken-and-egg problem as NodeId.UNREGISTERED will not be a CMS + // member. + // + // Fortunately, in both cases the placements for the metadata keyspace can be used as the source of truth a + // about CMS membership. As mentioned, during the PRE_INIT/INIT phase, the placement is hardcoded and during + // in-flight address changes a temporary lookup is constructed and the placements derived from that. + // See calculateCMSPlacement for details. + + if (epoch.isEqualOrBefore(Epoch.FIRST) || cmsLookup.isActive()) return isCMSMember(FBUtilities.getBroadcastAddressAndPort()); return fullCMSMemberIds().contains(localNodeId); @@ -260,9 +284,10 @@ public Set fullCMSMembers() if (fullCMSEndpoints == null) { + EndpointLookup lookup = endpointLookup(); fullCMSEndpoints = ImmutableSet.copyOf(cmsMembership.fullMembers() .stream() - .map(directory::endpoint) + .map(lookup::endpoint) .collect(Collectors.toSet())); } return fullCMSEndpoints; @@ -275,15 +300,54 @@ public EndpointsForRange fullCMSMembersAsReplicas() if (fullCMSReplicas == null) { + EndpointLookup lookup = endpointLookup(); EndpointsForRange.Builder builder = EndpointsForRange.builder(MetaStrategy.entireRange); for (NodeId nodeId : fullCMSMemberIds()) - builder.add(MetaStrategy.replica(directory.endpoint(nodeId))); + builder.add(MetaStrategy.replica(lookup.endpoint(nodeId))); fullCMSReplicas = builder.build(); } return fullCMSReplicas; } - private DataPlacement calculateCMSPlacement(DataPlacements placements, CMSMembership cms) + // Synchronization is probably not necessary as this should only be called by a log listener + public synchronized void refreshCMSLookup(ClusterMetadata prev, boolean fromSnapshot) + { + CMSLookup prevLookup = prev.cmsLookup; + CMSLookup proposedLookup = prevLookup.rebuild(prev, this, fromSnapshot); + // rebuild returns `this` if no changes + if (prevLookup != proposedLookup) + logger.info("Replacing CMSLookup, Current: {}, Proposed: {}", prevLookup, proposedLookup); + + // recalculate CMS placement using endpoint mappings from lookup + cmsDataPlacement = calculateCMSPlacement(placements, cmsMembership, proposedLookup); + + // We shouldn't need to null out the other lazily initialized CMS fields when we refresh the CMSLookup as the + // refresh is triggered by a precommit listener, meaning nothing should be accessing the ClusterMetadata + // instance yet. + cmsLookup = proposedLookup; + } + + // Synchronization is probably not necessary as this should only be called once, during startup + public synchronized boolean initCMSLookup(CMSLookup lookup) + { + logger.info("Initializing CMS lookup on ClusterMetadata at epoch {}", epoch.getEpoch()); + boolean isInitial = cmsLookup.isUninitialized(); + logger.debug("Current CMS lookup: {}, proposed: {}", cmsLookup, lookup); + if (isInitial) + { + cmsLookup = lookup; + cmsDataPlacement = calculateCMSPlacement(placements, cmsMembership, lookup); + // We need to null out these fields when we init the CMSLookup because post-commit listeners may have + // accessed them during log replay, which happens before this. + fullCMSReplicas = null; + fullCMSEndpoints = null; + } + else + logger.warn("Invalid CMSLookup state for initialization: {}", cmsLookup); + return isInitial; + } + + private DataPlacement calculateCMSPlacement(DataPlacements placements, CMSMembership cms, CMSLookup lookup) { if (epoch.isBefore(Epoch.FIRST) || schema.getKeyspaces().get(SchemaConstants.METADATA_KEYSPACE_NAME).isEmpty()) return DataPlacement.empty(); @@ -326,11 +390,17 @@ private DataPlacement calculateCMSPlacement(DataPlacements placements, CMSMember } else { - // Build a placement based on the CMS membership - return cms.toPlacement(directory); + // Build a placement based on the CMS membership, with any in-flight endpoint changes applied + EndpointLookup endpointLookup = lookup.isActive() ? lookup.asNodeLookup(directory) : directory; + return cms.toPlacement(endpointLookup); } } + public EndpointLookup endpointLookup() + { + return cmsLookup.isActive() ? cmsLookup.asNodeLookup(directory) : directory; + } + public DataPlacement placement(ReplicationParams params) { return params.isMeta() ? cmsDataPlacement : placements.get(params); diff --git a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java index 08fadb2aa820..fa323ad85b54 100644 --- a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java +++ b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java @@ -210,7 +210,8 @@ public static State state(ClusterMetadata metadata) Commit.Replicator replicator = CassandraRelevantProperties.TCM_USE_TEST_NO_OP_REPLICATOR.getBoolean() ? Commit.Replicator.NO_OP - : new Commit.DefaultReplicator(() -> log.metadata().directory); + : new Commit.DefaultReplicator(() -> log.metadata().directory, + () -> log.metadata().endpointLookup()); RemoteProcessor remoteProcessor = new RemoteProcessor(log, Discovery.instance::discoveredNodes); GossipProcessor gossipProcessor = new GossipProcessor(); diff --git a/src/java/org/apache/cassandra/tcm/Commit.java b/src/java/org/apache/cassandra/tcm/Commit.java index 7eda9b752300..ff62a814bc57 100644 --- a/src/java/org/apache/cassandra/tcm/Commit.java +++ b/src/java/org/apache/cassandra/tcm/Commit.java @@ -46,6 +46,7 @@ import org.apache.cassandra.tcm.log.Entry; import org.apache.cassandra.tcm.log.LogState; import org.apache.cassandra.tcm.membership.Directory; +import org.apache.cassandra.tcm.membership.EndpointLookup; import org.apache.cassandra.tcm.membership.NodeId; import org.apache.cassandra.tcm.membership.NodeVersion; import org.apache.cassandra.tcm.serialization.Version; @@ -436,10 +437,12 @@ public interface Replicator public static class DefaultReplicator implements Replicator { private final Supplier directorySupplier; + private final Supplier lookupSupplier; - public DefaultReplicator(Supplier directorySupplier) + public DefaultReplicator(Supplier directorySupplier, Supplier lookupSupplier) { this.directorySupplier = directorySupplier; + this.lookupSupplier = lookupSupplier; } public void send(Result result, InetAddressAndPort source) @@ -449,21 +452,19 @@ public void send(Result result, InetAddressAndPort source) Result.Success success = result.success(); Directory directory = directorySupplier.get(); + EndpointLookup lookup = lookupSupplier.get(); // Filter the log entries from the commit result for the purposes of replicating to members of the cluster // other than the original submitter. We only need to include the sublist of entries starting at the one // which was newly committed. We exclude entries before that one as the submitter may have been lagging and - // supplied a last known epoch arbitrarily in the past. We include entries after the first newly committed - // one as there may have been a new period automatically triggered and we'd like to push that out to all - // peers too. Of course, there may be other entries interspersed with these but it doesn't harm anything to - // include those too, it may simply be redundant. + // supplied a last known epoch arbitrarily in the past. LogState newlyCommitted = success.logState.retainFrom(success.epoch); assert !newlyCommitted.isEmpty() : String.format("Nothing to replicate after retaining epochs since %s from %s", success.epoch, success.logState); for (NodeId peerId : directory.peerIds()) { - InetAddressAndPort endpoint = directory.endpoint(peerId); + InetAddressAndPort endpoint = lookup.endpoint(peerId); boolean upgraded = directory.version(peerId).isUpgraded(); // Do not replicate to self and to the peer that has requested to commit this message if (endpoint.equals(FBUtilities.getBroadcastAddressAndPort()) || diff --git a/src/java/org/apache/cassandra/tcm/membership/Directory.java b/src/java/org/apache/cassandra/tcm/membership/Directory.java index 8d444c644386..1a4f825aa2d7 100644 --- a/src/java/org/apache/cassandra/tcm/membership/Directory.java +++ b/src/java/org/apache/cassandra/tcm/membership/Directory.java @@ -61,7 +61,7 @@ import static org.apache.cassandra.tcm.membership.NodeVersion.CURRENT; import static org.apache.cassandra.tcm.membership.NodeVersion.CURRENT_METADATA_VERSION; -public class Directory implements MetadataValue +public class Directory implements MetadataValue, EndpointLookup { public static final Serializer serializer = new Serializer(); diff --git a/src/java/org/apache/cassandra/tcm/membership/EndpointLookup.java b/src/java/org/apache/cassandra/tcm/membership/EndpointLookup.java new file mode 100644 index 000000000000..03840d948ff4 --- /dev/null +++ b/src/java/org/apache/cassandra/tcm/membership/EndpointLookup.java @@ -0,0 +1,26 @@ +/* + * 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.tcm.membership; + +import org.apache.cassandra.locator.InetAddressAndPort; + +public interface EndpointLookup +{ + InetAddressAndPort endpoint(NodeId id); +} From ccd89e4926a81986f7332145a8c728e928eb9a7d Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 2 Jun 2025 11:50:04 +0100 Subject: [PATCH 03/27] [CASSANDRA-20476] Perform rediscovery of CMS at startup if addresses change --- .../apache/cassandra/net/MessageDelivery.java | 6 +- src/java/org/apache/cassandra/net/Verb.java | 4 + .../cassandra/tcm/ClusterMetadataService.java | 2 +- .../org/apache/cassandra/tcm/Discovery.java | 74 +++++++--- .../org/apache/cassandra/tcm/Startup.java | 136 +++++++++++++++++- .../apache/cassandra/tcm/log/LocalLog.java | 23 +++ .../tcm/DiscoverySimulationTest.java | 2 +- 7 files changed, 223 insertions(+), 24 deletions(-) diff --git a/src/java/org/apache/cassandra/net/MessageDelivery.java b/src/java/org/apache/cassandra/net/MessageDelivery.java index 2e9155f71356..064dacb0e050 100644 --- a/src/java/org/apache/cassandra/net/MessageDelivery.java +++ b/src/java/org/apache/cassandra/net/MessageDelivery.java @@ -62,7 +62,7 @@ static Collection> fanoutAndWait(Messag @Override public void onResponse(Message msg) { - logger.info("Received a {} response from {}: {}", msg.verb(), msg.from(), msg.payload); + logger.debug("Received a {} response from {}: {}", msg.verb(), msg.from(), msg.payload); responses.add(Pair.create(msg.from(), msg.payload)); cdl.decrement(); } @@ -70,13 +70,13 @@ public void onResponse(Message msg) @Override public void onFailure(InetAddressAndPort from, RequestFailure reason) { - logger.info("Received failure in response to {} from {}: {}", verb, from, reason); + logger.debug("Received failure in response to {} from {}: {}", verb, from, reason); cdl.decrement(); } }; sendTo.forEach((ep) -> { - logger.info("Election for metadata migration sending {} ({}) to {}", verb, payload.toString(), ep); + logger.debug("Sending {} ({}) to {}", verb, payload.toString(), ep); messaging.sendWithCallback(Message.out(verb, payload), ep, callback); }); cdl.awaitUninterruptibly(timeout, timeUnit); diff --git a/src/java/org/apache/cassandra/net/Verb.java b/src/java/org/apache/cassandra/net/Verb.java index f0721fe997ff..d3b1adab1879 100644 --- a/src/java/org/apache/cassandra/net/Verb.java +++ b/src/java/org/apache/cassandra/net/Verb.java @@ -316,6 +316,10 @@ public enum Verb TCM_DISCOVER_REQ (813, P0, rpcTimeout, INTERNAL_METADATA, () -> NoPayload.serializer, () -> Discovery.instance.requestHandler, TCM_DISCOVER_RSP ), TCM_FETCH_PEER_LOG_RSP (818, P0, shortTimeout, FETCH_METADATA, MessageSerializers::logStateSerializer, RESPONSE_HANDLER ), TCM_FETCH_PEER_LOG_REQ (819, P0, rpcTimeout, FETCH_METADATA, () -> FetchPeerLog.serializer, () -> FetchPeerLog.Handler.instance, TCM_FETCH_PEER_LOG_RSP ), + TCM_DISCOVER_PEERS_RSP (820, P0, rpcTimeout, INTERNAL_METADATA, () -> Discovery.serializer, () -> ResponseVerbHandler.instance ), + TCM_DISCOVER_PEERS_REQ (821, P0, rpcTimeout, INTERNAL_METADATA, () -> NoPayload.serializer, () -> Discovery.instance.requestHandler, TCM_DISCOVER_PEERS_RSP), + TCM_DISCOVER_SURVEY_RSP(822, P0, rpcTimeout, INTERNAL_METADATA, () -> Discovery.nodeIdSerializer, () -> ResponseVerbHandler.instance ), + TCM_DISCOVER_SURVEY_REQ(823, P0, rpcTimeout, INTERNAL_METADATA, () -> NoPayload.serializer, () -> Discovery.instance.requestHandler, TCM_DISCOVER_SURVEY_RSP), INITIATE_DATA_MOVEMENTS_RSP (814, P1, rpcTimeout, MISC, () -> NoPayload.serializer, RESPONSE_HANDLER ), INITIATE_DATA_MOVEMENTS_REQ (815, P1, rpcTimeout, MISC, () -> DataMovement.serializer, () -> DataMovementVerbHandler.instance, INITIATE_DATA_MOVEMENTS_RSP ), diff --git a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java index fa323ad85b54..78c1b6c40d91 100644 --- a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java +++ b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java @@ -165,7 +165,7 @@ public static State state() return state(ClusterMetadata.current()); } - public static State state(ClusterMetadata metadata) + public static ClusterMetadataService.State state(ClusterMetadata metadata) { if (CassandraRelevantProperties.TCM_UNSAFE_BOOT_WITH_CLUSTERMETADATA.isPresent()) return RESET; diff --git a/src/java/org/apache/cassandra/tcm/Discovery.java b/src/java/org/apache/cassandra/tcm/Discovery.java index 99ca8907cc68..21990118d395 100644 --- a/src/java/org/apache/cassandra/tcm/Discovery.java +++ b/src/java/org/apache/cassandra/tcm/Discovery.java @@ -36,6 +36,7 @@ import org.slf4j.LoggerFactory; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.SystemKeyspace; import org.apache.cassandra.db.TypeSizes; import org.apache.cassandra.io.IVersionedSerializer; import org.apache.cassandra.io.util.DataInputPlus; @@ -47,6 +48,7 @@ import org.apache.cassandra.net.MessagingService; import org.apache.cassandra.net.NoPayload; import org.apache.cassandra.net.Verb; +import org.apache.cassandra.tcm.membership.NodeId; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.Pair; @@ -63,6 +65,28 @@ public class Discovery public static final Discovery instance = new Discovery(); public static final Serializer serializer = new Serializer(); + // TODO add this to MessageSerializers properly or define a real response format + public static final IVersionedSerializer nodeIdSerializer = new IVersionedSerializer<>() + { + @Override + public void serialize(NodeId t, DataOutputPlus out, int version) throws IOException + { + out.writeUnsignedVInt32(t.id()); + } + + @Override + public NodeId deserialize(DataInputPlus in, int version) throws IOException + { + int id = in.readUnsignedVInt32(); + return new NodeId(id); + } + + @Override + public long serializedSize(NodeId t, int version) + { + return TypeSizes.sizeofUnsignedVInt(t.id()); + } + }; public final IVerbHandler requestHandler; private final Set discovered = new ConcurrentSkipListSet<>(); @@ -94,12 +118,14 @@ public Discovery(Supplier messaging, Supplier candidates = new HashSet<>(); if (initiator != null) @@ -148,12 +176,11 @@ public DiscoveredNodes discoverOnce(InetAddressAndPort initiator, long timeout, else candidates.addAll(discovered); - if (candidates.isEmpty()) - candidates.addAll(seeds.get()); - + candidates.addAll(seeds.get()); candidates.remove(self); - Collection> responses = MessageDelivery.fanoutAndWait(messaging.get(), candidates, Verb.TCM_DISCOVER_REQ, NoPayload.noPayload, timeout, timeUnit); + Verb verb = allPeers ? Verb.TCM_DISCOVER_PEERS_REQ : Verb.TCM_DISCOVER_REQ; + Collection> responses = MessageDelivery.fanoutAndWait(messaging.get(), candidates, verb, NoPayload.noPayload, timeout, timeUnit); for (Pair discoveredNodes : responses) { @@ -179,19 +206,30 @@ private final class DiscoveryRequestHandler implements IVerbHandler @Override public void doVerb(Message message) { + discovered.add(message.from()); Set cms = ClusterMetadata.current().fullCMSMembers(); - logger.debug("Responding to discovery request from {}: {}", message.from(), cms); - DiscoveredNodes discoveredNodes; - if (!cms.isEmpty()) - discoveredNodes = new DiscoveredNodes(cms, DiscoveredNodes.Kind.CMS_ONLY); - else + switch (message.verb()) { - discovered.add(message.from()); - discoveredNodes = new DiscoveredNodes(new HashSet<>(discovered), DiscoveredNodes.Kind.KNOWN_PEERS); + case TCM_DISCOVER_REQ: + logger.trace("Responding to discovery request from {}: {}", message.from(), cms); + if (!cms.isEmpty()) + discoveredNodes = new DiscoveredNodes(cms, DiscoveredNodes.Kind.CMS_ONLY); + else + discoveredNodes = new DiscoveredNodes(new HashSet<>(discovered), DiscoveredNodes.Kind.KNOWN_PEERS); + messaging.get().send(message.responseWith(discoveredNodes), message.from()); + break; + case TCM_DISCOVER_PEERS_REQ: + logger.trace("Responding to {} request from {}", message.verb(), message.from()); + discoveredNodes = new DiscoveredNodes(new HashSet<>(discovered), DiscoveredNodes.Kind.KNOWN_PEERS); + messaging.get().send(message.responseWith(discoveredNodes), message.from()); + break; + case TCM_DISCOVER_SURVEY_REQ: + logger.trace("Responding to {} request from {}", message.verb(), message.from()); + NodeId id = NodeId.fromUUID(SystemKeyspace.getLocalHostId()); + messaging.get().send(message.responseWith(id), message.from()); + break; } - - messaging.get().send(message.responseWith(discoveredNodes), message.from()); } } diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index f9a4313afa71..b87cca6614f7 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -19,7 +19,9 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -52,7 +54,10 @@ import org.apache.cassandra.gms.NewGossiper; import org.apache.cassandra.gms.VersionedValue; import org.apache.cassandra.locator.InetAddressAndPort; +import org.apache.cassandra.net.MessageDelivery; import org.apache.cassandra.net.MessagingService; +import org.apache.cassandra.net.NoPayload; +import org.apache.cassandra.net.Verb; import org.apache.cassandra.schema.DistributedSchema; import org.apache.cassandra.schema.KeyspaceMetadata; import org.apache.cassandra.schema.Keyspaces; @@ -118,7 +123,16 @@ public static void initialize(Set seeds, case NORMAL: logger.info("Initializing as non CMS node"); initializeAsNonCmsNode(wrapProcessor); + ClusterMetadataService.instance().log().pause(); initMessaging.run(); + UUID localHostId = SystemKeyspace.getLocalHostId(); + // If null, this node has not yet registered so there will be more nothing to do here + if (localHostId != null) + { + NodeId nodeId = NodeId.fromUUID(localHostId); + initializeCMSLookup(nodeId, ClusterMetadata.current()); + } + ClusterMetadataService.instance().log().unpause(); break; case VOTE: logger.info("Initializing for discovery"); @@ -183,7 +197,6 @@ public static void initializeAsNonCmsNode(Function wrapPro ClusterMetadataService::state, logSpec)); ClusterMetadataService.instance().log().ready(); - NodeId nodeId = ClusterMetadata.current().myNodeId(); UUID currentHostId = SystemKeyspace.getLocalHostId(); if (nodeId != NodeId.UNREGISTERED && !Objects.equals(nodeId.toUUID(), currentHostId)) @@ -202,6 +215,127 @@ public static void initializeAsNonCmsNode(Function wrapPro } } + + /** + * If the broadcast address of this node has changed, we must verify the endpoints it knows for + * the members of the CMS are still reachable and valid. This is necessary for the node to submit + * a STARTUP transformation which updates its broadcast address in ClusterMetadata. + * + * If the node is itself a CMS member, it is also a requirement to be able to contact a + * majority of the other CMS members in order to perform the serial reads and writes which + * constitute committing to and fetching from the distributed metadata log. + * + * To do this, we use a simple protocol: + * 1. For each CMS member in our replayed ClusterMetadata, ping the associated broadcast address + * to query for id of the node at that address. This determines whether the endpoint still + * belongs to that same node (which is/was a CMS member). + * 2. While we don't have confirmed current addresses for a majority of CMS nodes: + * 2a. Run discovery to locate as many peer addresses as possible. + * 2b. Query every discovered endpoint and ask for its node id. + * If we still don't have confirmed addresses for a majority of CMS members, go to 2a and + * repeat as peers may themselves still be starting up and so may have become discoverable. + * + * This process builds up a mapping of id -> current address for CMS members which can then be + * used to construct a set of temporary redirects between addresses according to ClusterMetadata + * and the newly discovered ones. + * + * As each CMS node with a changed address goes through the startup process, it will commit its + * STARTUP transformation and the new broadcast address will be found in ClusterMetadata. A log + * listener is used to react to these transformations by removing redundant address overrides + * as they are enacted. + * + * @param nodeId derived from the persisted id of this node from the system.peers table + * @param replayed current ClusterMetadata after replaying the metadata log for startup + */ + private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadata replayed) + { + InetAddressAndPort oldAddress = replayed.directory.endpoint(nodeId); + InetAddressAndPort newAddress = FBUtilities.getBroadcastAddressAndPort(); + if (newAddress.equals(oldAddress)) + return replayed; + + Map previousCMS = new HashMap<>(); + replayed.fullCMSMemberIds().forEach(id -> previousCMS.put(id, replayed.directory.endpoint(id))); + Map confirmedCMS = new HashMap<>(); + + Set candidates = new HashSet<>(previousCMS.values()); + candidates.add(newAddress); + + int maxRounds = 5; + int currentRound = 0; + long roundTimeNanos = Math.min(TimeUnit.SECONDS.toNanos(4), + DatabaseDescriptor.getDiscoveryTimeout(TimeUnit.NANOSECONDS) / maxRounds); + // TODO a non-CMS node only needs to be able to contact a single CMS member to commit its STARTUP + int quorum = (previousCMS.size() / 2) + 1; + logger.info("Running survey and discovery for CMS nodes {} (quorum = {})", previousCMS, quorum); + while (confirmedCMS.size() < quorum && currentRound < maxRounds) + { + logger.info("In round {} sending survey to {}", currentRound, candidates); + Collection> surveyed = + MessageDelivery.fanoutAndWait(MessagingService.instance(), + candidates, + Verb.TCM_DISCOVER_SURVEY_REQ, + NoPayload.noPayload, + roundTimeNanos, + TimeUnit.NANOSECONDS); + logger.info("Survey of {} discovered {}", candidates, surveyed); + surveyed.forEach(pair -> { + if (previousCMS.containsKey(pair.right)) + confirmedCMS.put(pair.right, pair.left); + }); + + logger.info("Confirmed CMS members {}", confirmedCMS); + if (confirmedCMS.size() < quorum || (previousCMS.size() == 1 && confirmedCMS.containsKey(nodeId))) + { + // In the single node CMS case, run discovery simply to propagate this node's new address to the rest of + // the cluster via the seeds & discovery meshing. Otherwise, if every node has a new address the non-CMS + // members have no way to discover the CMS and it has no way to know the new places to push updates to. + logger.info("Running discovery round; either CMS quorum was not confirmed or this is the only CMS member"); + Discovery.DiscoveredNodes nodes = Discovery.instance.discover(5, true); + candidates.addAll(nodes.nodes()); + logger.info("Rediscovery completed, discovered nodes: {}", nodes); + } + currentRound++; + } + + if (confirmedCMS.size() >= quorum) + { + logger.info("Identified a quorum of CMS members (found {}, required {}).", confirmedCMS.size(), quorum); + if (confirmedCMS.values().containsAll(previousCMS.values())) + { + logger.info("No endpoint changes found for CMS members"); + return replayed; + } + else + { + logger.info("Applying temporary address overrides for uncommitted CMS endpoint changes"); + CMSLookup.InitialBuilder builder = CMSLookup.builder(replayed); + for (NodeId confirmed : confirmedCMS.keySet()) + { + InetAddressAndPort prev = previousCMS.get(confirmed); + InetAddressAndPort next = confirmedCMS.get(confirmed); + if (!next.equals(prev)) + { + logger.info("Added override for {}, ({} -> {})", confirmed, previousCMS.get(confirmed), confirmedCMS.get(confirmed)); + builder = builder.withOverride(confirmed, previousCMS.get(confirmed), confirmedCMS.get(confirmed)); + } + } + if (replayed.initCMSLookup(builder.build())) + { + ClusterMetadataService.instance().log().addListener(new CMSLookup.LogListener()); + return replayed; + } + + throw new RuntimeException("Could not initialize CMS lookup"); + } + } + else + { + throw new RuntimeException(String.format("Unable to identify a quorum of CMS members (found %s, required %s)", + confirmedCMS.size(), quorum)); + } + } + public static void scrubDataDirectories(ClusterMetadata metadata) throws StartupException { // clean up debris in the rest of the keyspaces diff --git a/src/java/org/apache/cassandra/tcm/log/LocalLog.java b/src/java/org/apache/cassandra/tcm/log/LocalLog.java index 9e283b324491..02a3c60b300b 100644 --- a/src/java/org/apache/cassandra/tcm/log/LocalLog.java +++ b/src/java/org/apache/cassandra/tcm/log/LocalLog.java @@ -103,6 +103,8 @@ public abstract class LocalLog implements Closeable // notification to them all. private final AtomicBoolean replayComplete = new AtomicBoolean(); + private final AtomicBoolean paused = new AtomicBoolean(); + public static LogSpec logSpec() { return new LogSpec(); @@ -378,6 +380,21 @@ public void append(Collection entries) } } + /** + * Temporarily halts processing of the pending buffer. While paused, entries may be appended to the buffer but + * they will not be enacted. Currently only used sparingly during startup if any address changes for CMS members are + * detected. In this case, we pause just while a mapping from old to new addresses is being built. + */ + public void pause() + { + paused.set(true); + } + + public void unpause() + { + paused.set(false); + } + public void append(Entry entry) { maybeAppend(entry); @@ -469,6 +486,12 @@ private Entry peek() */ void processPendingInternal() { + if (paused.get()) + { + logger.trace("Entry processing is paused, returning without scanning pending buffer"); + return; + } + while (true) { Entry pendingEntry = peek(); diff --git a/test/unit/org/apache/cassandra/tcm/DiscoverySimulationTest.java b/test/unit/org/apache/cassandra/tcm/DiscoverySimulationTest.java index 85fa61ed9b31..5f5024c7aef5 100644 --- a/test/unit/org/apache/cassandra/tcm/DiscoverySimulationTest.java +++ b/test/unit/org/apache/cassandra/tcm/DiscoverySimulationTest.java @@ -101,7 +101,7 @@ public void discoveryTest() throws Throwable Map> futures = new HashMap<>(); nodes.forEach((addr, discovery) -> { - futures.put(addr, CompletableFuture.supplyAsync(() -> discovery.discover(5), executor)); + futures.put(addr, CompletableFuture.supplyAsync(() -> discovery.discover(5, false), executor)); }); Map discovered = new HashMap<>(); From b51da1b23ec3e3a343f87cdbfc5d10ed112df16f Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Wed, 25 Jun 2025 18:40:59 +0100 Subject: [PATCH 04/27] [CASSANDRA-20476] Attempt to wait for all address changes to be enacted before allowing startup to proceed --- .../apache/cassandra/gms/FailureDetector.java | 5 +++ .../cassandra/net/ResponseVerbHandler.java | 2 + .../org/apache/cassandra/tcm/Startup.java | 39 +++++++++++++++++-- .../test/log/DiscoverNewCMSTest.java | 16 ++++++-- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/java/org/apache/cassandra/gms/FailureDetector.java b/src/java/org/apache/cassandra/gms/FailureDetector.java index 13d6266ca039..34f53508093a 100644 --- a/src/java/org/apache/cassandra/gms/FailureDetector.java +++ b/src/java/org/apache/cassandra/gms/FailureDetector.java @@ -328,6 +328,11 @@ public boolean isAlive(InetAddressAndPort ep) // registration via the metadata log, or a full gossip round). This is perfectly harmless, so no need to log // an error in that case. ClusterMetadata metadata = ClusterMetadata.current(); + if (metadata.cmsLookup.isActive() && metadata.fullCMSMembers().contains(ep)) + { + logger.trace("Found endpoint {} in active CMS lookup, assuming it is alive", ep); + return true; + } if (!metadata.directory.allJoinedEndpoints().contains(ep) && !metadata.fullCMSMembers().contains(ep)) logger.error("Unknown endpoint: " + ep, new UnknownEndpointException(ep)); } diff --git a/src/java/org/apache/cassandra/net/ResponseVerbHandler.java b/src/java/org/apache/cassandra/net/ResponseVerbHandler.java index 215b575c1002..d44c9510f127 100644 --- a/src/java/org/apache/cassandra/net/ResponseVerbHandler.java +++ b/src/java/org/apache/cassandra/net/ResponseVerbHandler.java @@ -48,6 +48,8 @@ class ResponseVerbHandler implements IVerbHandler Verb.TCM_REPLICATION, Verb.TCM_NOTIFY_RSP, Verb.TCM_DISCOVER_RSP, + Verb.TCM_DISCOVER_PEERS_RSP, + Verb.TCM_DISCOVER_SURVEY_RSP, Verb.TCM_INIT_MIG_RSP); // We skip epoch catchup for PaxosV2 verbs, since we are using PaxosV2 to serially read the log. diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index b87cca6614f7..f52d6c6bb287 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -122,17 +122,47 @@ public static void initialize(Set seeds, break; case NORMAL: logger.info("Initializing as non CMS node"); + // note: if this node was a member of the CMS, log replay will put it back into that state initializeAsNonCmsNode(wrapProcessor); - ClusterMetadataService.instance().log().pause(); - initMessaging.run(); UUID localHostId = SystemKeyspace.getLocalHostId(); // If null, this node has not yet registered so there will be more nothing to do here if (localHostId != null) { NodeId nodeId = NodeId.fromUUID(localHostId); - initializeCMSLookup(nodeId, ClusterMetadata.current()); + ClusterMetadata replayed = ClusterMetadata.current(); + InetAddressAndPort oldAddress = replayed.directory.endpoint(nodeId); + InetAddressAndPort newAddress = FBUtilities.getBroadcastAddressAndPort(); + if (!newAddress.equals(oldAddress)) + { + // Build temporary mappings for addressing CMS nodes who's addresses have been + // changed but not yet committed via the CMS or not yet been enacted locally. + ClusterMetadataService.instance().log().pause(); + initMessaging.run(); + initializeCMSLookup(nodeId, replayed); + ClusterMetadataService.instance().log().unpause(); + + logger.info("Detected change in local node addresses, committing update to Cluster Metadata Service"); + Transformation transform = new org.apache.cassandra.tcm.transformations.Startup(nodeId, + NodeAddresses.current(), + NodeVersion.CURRENT); + replayed = ClusterMetadataService.instance().commit(transform); + + // Wait for any CMS members which have uncommitted address changes to commit them and + // us to enact them. + while (replayed.cmsLookup.isActive()) + { + logger.info("Waiting for pending CMS address changes to complete {}", replayed.cmsLookup); + TimeUnit.MILLISECONDS.sleep(1000); // TODO make configurable? + replayed = ClusterMetadata.current(); + } + } + logger.info("CMS address changes complete, current epoch is {}", replayed.epoch.getEpoch()); + } + else + { + // nothing to do, so just initialise messaging + initMessaging.run(); } - ClusterMetadataService.instance().log().unpause(); break; case VOTE: logger.info("Initializing for discovery"); @@ -282,6 +312,7 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat surveyed.forEach(pair -> { if (previousCMS.containsKey(pair.right)) confirmedCMS.put(pair.right, pair.left); + }); logger.info("Confirmed CMS members {}", confirmedCMS); diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java b/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java index f73465aa476a..3474ea9838b6 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java @@ -103,12 +103,20 @@ public void multiNodeCMSAllAddressesChangeTest() throws IOException, ExecutionEx private void test(Cluster cluster, int cmsSize) throws IOException, ExecutionException, InterruptedException { ExecutorService executor = Executors.newFixedThreadPool(cluster.size()); + // Some fetchCMSLog operations might temporarily fail and be retried during address changes + String[] expectedErrors = new String[] { + "Cannot achieve consistency level SERIAL.*", + "Queried for epoch Epoch\\{epoch=\\d+\\}, but could not catch up.*" + }; cluster.setUncaughtExceptionsFilter((node, t) -> { Throwable rootCause = Throwables.getRootCause(t); - // Some fetchCMSLog operations might temporarily fail and be retried during address changes - return rootCause.getMessage() != null - && rootCause.getMessage().startsWith("Cannot achieve consistency level SERIAL"); - + if (rootCause.getMessage() == null) + return false; + String message = rootCause.getMessage(); + for (String expected : expectedErrors) + if (message.matches(expected)) + return true; + return false; }); cluster.startup(); init(cluster); From 8ae56a4972ab7290390ec444a9ffd62674ebb287 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Tue, 24 Jun 2025 13:09:55 +0100 Subject: [PATCH 05/27] [CASSANDRA-20476] Some minor logging additions --- src/java/org/apache/cassandra/tcm/Commit.java | 2 +- .../org/apache/cassandra/tcm/transformations/Startup.java | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/tcm/Commit.java b/src/java/org/apache/cassandra/tcm/Commit.java index ff62a814bc57..56e74ce91ff4 100644 --- a/src/java/org/apache/cassandra/tcm/Commit.java +++ b/src/java/org/apache/cassandra/tcm/Commit.java @@ -374,8 +374,8 @@ static class Handler implements IVerbHandler public void doVerb(Message message) throws IOException { - checkCMSState(); logger.info("Received commit request {} from {}", message.payload, message.from()); + checkCMSState(); // Reduce our local retry deadline by write_rpc_timeout so we exhaust retries and return an // explicit failure response to the sender before their per-message callback fires to avoid // all the non-CMS nodes being synchronized on their CMS await timeouts expiring at the same time. diff --git a/src/java/org/apache/cassandra/tcm/transformations/Startup.java b/src/java/org/apache/cassandra/tcm/transformations/Startup.java index 00b402352599..caa2e53ce5bf 100644 --- a/src/java/org/apache/cassandra/tcm/transformations/Startup.java +++ b/src/java/org/apache/cassandra/tcm/transformations/Startup.java @@ -22,6 +22,9 @@ import java.util.Map; import java.util.Objects; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import org.apache.cassandra.io.util.DataInputPlus; import org.apache.cassandra.io.util.DataOutputPlus; import org.apache.cassandra.tcm.ClusterMetadata; @@ -40,6 +43,7 @@ public class Startup implements Transformation { + private static final Logger logger = LoggerFactory.getLogger(Startup.class); public static final Serializer serializer = new Serializer(); private final NodeId nodeId; private final NodeVersion nodeVersion; @@ -123,6 +127,7 @@ public static void maybeExecuteStartupTransformation(NodeId localNodeId) if (!Objects.equals(directory.addresses.get(localNodeId), NodeAddresses.current()) || !Objects.equals(directory.versions.get(localNodeId), NodeVersion.CURRENT)) { + logger.info("Detected change in node addresses or version, committing updates to Cluster Metadata Service"); ClusterMetadataService.instance() .commit(new Startup(localNodeId, NodeAddresses.current(), NodeVersion.CURRENT)); } From c18ba33d17c9a270bca8998f9058cfbda9159ec7 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Tue, 5 Aug 2025 19:24:29 +0100 Subject: [PATCH 06/27] [CASSANDRA-20476] Make sure to start messaging service --- src/java/org/apache/cassandra/tcm/Startup.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index f52d6c6bb287..8803be322153 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -126,7 +126,11 @@ public static void initialize(Set seeds, initializeAsNonCmsNode(wrapProcessor); UUID localHostId = SystemKeyspace.getLocalHostId(); // If null, this node has not yet registered so there will be more nothing to do here - if (localHostId != null) + if (localHostId == null) + { + initMessaging.run(); + } + else { NodeId nodeId = NodeId.fromUUID(localHostId); ClusterMetadata replayed = ClusterMetadata.current(); @@ -155,13 +159,13 @@ public static void initialize(Set seeds, TimeUnit.MILLISECONDS.sleep(1000); // TODO make configurable? replayed = ClusterMetadata.current(); } + logger.info("Any in flight CMS address changes have been processed, current epoch is {}", replayed.epoch.getEpoch()); + } + else + { + // nothing more to do, so just initialize messaging + initMessaging.run(); } - logger.info("CMS address changes complete, current epoch is {}", replayed.epoch.getEpoch()); - } - else - { - // nothing to do, so just initialise messaging - initMessaging.run(); } break; case VOTE: From 3ffee0dd86f4c907716436b5d8ae26d6831d1d11 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Thu, 22 Jan 2026 19:15:39 +0000 Subject: [PATCH 07/27] [CASSANDRA-20476] Don't attempt to catch up from peers or CMS while log processing is suspended --- .../cassandra/tcm/ClusterMetadataService.java | 26 ++++++++++++++++++- .../apache/cassandra/tcm/PeerLogFetcher.java | 1 + .../apache/cassandra/tcm/log/LocalLog.java | 7 ++++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java index 78c1b6c40d91..e3f612e63405 100644 --- a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java +++ b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java @@ -826,10 +826,15 @@ public ClusterMetadata fetchLogFromCMS(Epoch awaitAtLeast) return metadata; Epoch ourEpoch = metadata.epoch; - if (ourEpoch.isEqualOrAfter(awaitAtLeast)) return metadata; + if (log.isPaused()) + { + logger.debug("Fetch metadata log from CMS was requested, but log processing is paused"); + return metadata; + } + Retry deadline = Retry.untilElapsed(getCmsAwaitTimeout().to(TimeUnit.NANOSECONDS), TCMMetrics.instance.fetchLogRetries); // responses for ALL withhout knowing we have pending metadata = processor.fetchLogAndWait(awaitAtLeast, deadline); @@ -864,6 +869,12 @@ public Future fetchLogFromPeerAsync(InetAddressAndPort from, Ep awaitAtLeast.isBefore(Epoch.FIRST)) return ImmediateFuture.success(current); + if (log.isPaused()) + { + logger.debug("Fetch metadata log from peer was requested, but log processing is paused"); + return ImmediateFuture.success(current); + } + return peerLogFetcher.asyncFetchLog(from, awaitAtLeast); } @@ -890,6 +901,13 @@ private ClusterMetadata fetchLogFromPeer(ClusterMetadata metadata, InetAddressAn Epoch before = metadata.epoch; if (before.isEqualOrAfter(awaitAtLeast)) return metadata; + + if (log.isPaused()) + { + logger.debug("Fetch metadata log from peer was requested, but log processing is paused"); + return metadata; + } + return peerLogFetcher.fetchLogEntriesAndWait(from, awaitAtLeast); } @@ -945,6 +963,12 @@ public ClusterMetadata fetchLogFromPeerOrCMS(ClusterMetadata metadata, InetAddre if (awaitAtLeast.isBefore(Epoch.FIRST) || FBUtilities.getBroadcastAddressAndPort().equals(from)) return metadata; + if (log.isPaused()) + { + logger.debug("Fetch metadata log from peer or CMS was requested, but log processing is paused"); + return metadata; + } + Epoch before = metadata.epoch; if (before.isEqualOrAfter(awaitAtLeast)) return metadata; diff --git a/src/java/org/apache/cassandra/tcm/PeerLogFetcher.java b/src/java/org/apache/cassandra/tcm/PeerLogFetcher.java index 4a131954cb06..1f01803d9ff2 100644 --- a/src/java/org/apache/cassandra/tcm/PeerLogFetcher.java +++ b/src/java/org/apache/cassandra/tcm/PeerLogFetcher.java @@ -69,6 +69,7 @@ public ClusterMetadata fetchLogEntriesAndWait(InetAddressAndPort remote, Epoch a catch (ExecutionException | TimeoutException e) { logger.warn("Could not fetch log entries from peer, remote = {}, await = {}", remote, awaitAtleast); + logger.debug("Exception while fetching log entries from peer, remote = {}", remote, e); } return metadata; } diff --git a/src/java/org/apache/cassandra/tcm/log/LocalLog.java b/src/java/org/apache/cassandra/tcm/log/LocalLog.java index 02a3c60b300b..4622fcad5573 100644 --- a/src/java/org/apache/cassandra/tcm/log/LocalLog.java +++ b/src/java/org/apache/cassandra/tcm/log/LocalLog.java @@ -395,6 +395,11 @@ public void unpause() paused.set(false); } + public boolean isPaused() + { + return paused.get(); + } + public void append(Entry entry) { maybeAppend(entry); @@ -488,7 +493,7 @@ void processPendingInternal() { if (paused.get()) { - logger.trace("Entry processing is paused, returning without scanning pending buffer"); + logger.info("Metadata log entry processing is paused, returning without scanning pending buffer"); return; } From d918d3e8b32a7e802cc6de5677deb00b66e9ee1c Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 16 Feb 2026 13:57:59 +0000 Subject: [PATCH 08/27] [CASSANDRA-20476] Remove unused CMSLookup.addressMap --- src/java/org/apache/cassandra/tcm/CMSLookup.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/CMSLookup.java b/src/java/org/apache/cassandra/tcm/CMSLookup.java index 6713b7f75a55..c8b85cd3b203 100644 --- a/src/java/org/apache/cassandra/tcm/CMSLookup.java +++ b/src/java/org/apache/cassandra/tcm/CMSLookup.java @@ -24,8 +24,6 @@ import java.util.function.Predicate; import java.util.stream.Collectors; -import com.google.common.collect.BiMap; -import com.google.common.collect.HashBiMap; import com.google.common.collect.Maps; import org.slf4j.Logger; @@ -50,7 +48,6 @@ public static InitialBuilder builder(ClusterMetadata metadata) } private final Map> overrides; - private final BiMap addressMap; private final Epoch lastModified; private final State state; @@ -58,13 +55,9 @@ private CMSLookup(State state, Epoch epoch, Map> e : overrides.entrySet()) - { this.overrides.put(e.getKey(), e.getValue()); - this.addressMap.put(e.getValue().left, e.getValue().right); - } } public boolean isUninitialized() @@ -77,12 +70,6 @@ public boolean isActive() return state == State.ACTIVE; } - public InetAddressAndPort getAddressOverride(NodeId id) - { - Pair override = overrides.get(id); - return override != null ? override.right : null; - } - public EndpointLookup asNodeLookup(EndpointLookup lookup) { return new EndpointLookup() @@ -146,7 +133,6 @@ public String toString() "state=" + state + ", epoch=" + lastModified + ", overrides=" + overrides + - ", addressMap=" + addressMap + '}'; } From 46e689014e9a6a0c31add402e5a5953922ede8e6 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 16 Feb 2026 14:32:41 +0000 Subject: [PATCH 09/27] [CASSANDRA-20476] Remove unnecessary map retrievals --- src/java/org/apache/cassandra/tcm/Startup.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index 8803be322153..e9f148caa2bb 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -351,8 +351,8 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat InetAddressAndPort next = confirmedCMS.get(confirmed); if (!next.equals(prev)) { - logger.info("Added override for {}, ({} -> {})", confirmed, previousCMS.get(confirmed), confirmedCMS.get(confirmed)); - builder = builder.withOverride(confirmed, previousCMS.get(confirmed), confirmedCMS.get(confirmed)); + logger.info("Added override for {}, ({} -> {})", confirmed, prev, next); + builder = builder.withOverride(confirmed, prev, next); } } if (replayed.initCMSLookup(builder.build())) From 774780896ebb4697fcc5bb511f40fad5d039fb60 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 16 Feb 2026 15:38:00 +0000 Subject: [PATCH 10/27] [CASSANDRA-20476] Make CMSLookup.overrides an ImmutableMap --- .../org/apache/cassandra/tcm/CMSLookup.java | 48 ++++++++++++------- .../org/apache/cassandra/tcm/Startup.java | 11 ++++- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/CMSLookup.java b/src/java/org/apache/cassandra/tcm/CMSLookup.java index c8b85cd3b203..2a1d1db21901 100644 --- a/src/java/org/apache/cassandra/tcm/CMSLookup.java +++ b/src/java/org/apache/cassandra/tcm/CMSLookup.java @@ -22,9 +22,8 @@ import java.util.Map; import java.util.Objects; import java.util.function.Predicate; -import java.util.stream.Collectors; -import com.google.common.collect.Maps; +import com.google.common.collect.ImmutableMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,23 +40,21 @@ public class CMSLookup public enum State { PRE_INIT, ACTIVE, RETIRED }; - public final static CMSLookup NO_OP = new CMSLookup(State.PRE_INIT, Epoch.EMPTY, new HashMap<>()); + public final static CMSLookup NO_OP = new CMSLookup(State.PRE_INIT, Epoch.EMPTY, ImmutableMap.of()); public static InitialBuilder builder(ClusterMetadata metadata) { return new InitialBuilder(metadata); } - private final Map> overrides; + private final ImmutableMap> overrides; private final Epoch lastModified; private final State state; - private CMSLookup(State state, Epoch epoch, Map> overrides) + private CMSLookup(State state, Epoch epoch, ImmutableMap> overrides) { this.state = state; this.lastModified = epoch; - this.overrides = Maps.newHashMapWithExpectedSize(overrides.size()); - for (Map.Entry> e : overrides.entrySet()) - this.overrides.put(e.getKey(), e.getValue()); + this.overrides = overrides; } public boolean isUninitialized() @@ -98,8 +95,8 @@ public CMSLookup rebuild(ClusterMetadata prev, ClusterMetadata next, boolean fro return this; // Filters from the override list those which are no longer necessary as a transformation has now - // replaced the old address with the new one for that node id - Predicate>> overrideNowEnacted = entry -> { + // replaced the old address with a new one for that node id + Predicate>> overrideRequired = entry -> { NodeId nodeId = entry.getKey(); if (!Objects.equals(prev.directory.getNodeAddresses(nodeId), next.directory.getNodeAddresses(nodeId))) { @@ -115,13 +112,21 @@ public CMSLookup rebuild(ClusterMetadata prev, ClusterMetadata next, boolean fro return true; }; - logger.debug("Current endpoint overrides: {}", overrides); - Map> nextOverrides - = overrides.entrySet() - .stream() - .filter(overrideNowEnacted) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - logger.debug("Proposed endpoint overrides: {}", nextOverrides); + logger.info("Current endpoint overrides: {}", overrides); + ImmutableMap.Builder> builder = ImmutableMap.builder(); + for (Map.Entry> override : overrides.entrySet()) + { + if(overrideRequired.test(override)) + builder.put(override.getKey(), override.getValue()); + } + + ImmutableMap> nextOverrides = builder.build(); + if (nextOverrides.equals(overrides)) + { + logger.info("No changes to endpoint overrides detected"); + return this; + } + logger.info("Proposed endpoint overrides: {}", nextOverrides); State state = nextOverrides.isEmpty() ? State.RETIRED : State.ACTIVE; return new CMSLookup(state, next.epoch, nextOverrides); } @@ -153,9 +158,16 @@ public InitialBuilder withOverride(NodeId id, InetAddressAndPort originalAddress return this; } + public boolean hasOverrides() + { + return !overrides.isEmpty(); + } + public CMSLookup build() { - return new CMSLookup(State.ACTIVE, epoch, overrides); + if (overrides.isEmpty()) + throw new IllegalStateException("No overrides detected"); + return new CMSLookup(State.ACTIVE, epoch, ImmutableMap.copyOf(overrides)); } } diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index e9f148caa2bb..83ea362dc87c 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -355,13 +355,20 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat builder = builder.withOverride(confirmed, prev, next); } } + + if (!builder.hasOverrides()) + { + logger.info("No overrides required for CMS members"); + return replayed; + } + if (replayed.initCMSLookup(builder.build())) { ClusterMetadataService.instance().log().addListener(new CMSLookup.LogListener()); return replayed; } - - throw new RuntimeException("Could not initialize CMS lookup"); + else + throw new RuntimeException("Could not initialize CMS lookup"); } } else From 12bda58792aca38075a1eee53c042500a4217266 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 16 Feb 2026 16:04:40 +0000 Subject: [PATCH 11/27] [CASSANDRA-20476] When CMSLookup is RETIRED, remove log listener --- src/java/org/apache/cassandra/tcm/CMSLookup.java | 5 +++++ src/java/org/apache/cassandra/tcm/Startup.java | 1 + .../cassandra/distributed/test/log/DiscoverNewCMSTest.java | 5 +++++ 3 files changed, 11 insertions(+) diff --git a/src/java/org/apache/cassandra/tcm/CMSLookup.java b/src/java/org/apache/cassandra/tcm/CMSLookup.java index 2a1d1db21901..4b50c074e243 100644 --- a/src/java/org/apache/cassandra/tcm/CMSLookup.java +++ b/src/java/org/apache/cassandra/tcm/CMSLookup.java @@ -178,6 +178,11 @@ public void notifyPreCommit(ClusterMetadata prev, ClusterMetadata next, boolean { logger.debug("Reevaluating CMSLookup from {} at epoch {}", prev.epoch, next.epoch); next.refreshCMSLookup(prev, fromSnapshot); + if (next.cmsLookup.state == State.RETIRED) + { + logger.info("CMSLookup state is RETIRED, removing log listener"); + ClusterMetadataService.instance().log().removeListener(this); + } } } } diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index 83ea362dc87c..e8c360a20542 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -364,6 +364,7 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat if (replayed.initCMSLookup(builder.build())) { + logger.info("Adding CMS lookup log listener"); ClusterMetadataService.instance().log().addListener(new CMSLookup.LogListener()); return replayed; } diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java b/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java index 3474ea9838b6..7865540ed9a6 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java @@ -197,6 +197,11 @@ private void test(Cluster cluster, int cmsSize) throws IOException, ExecutionExc assertEquals("Check failed on instance " + inst.config().num(), peer.right, fromInst); } } + + cluster.schemaChange("Create keyspace ks1 with replication = {'class': 'SimpleStrategy', 'replication_factor': 1}"); + cluster.schemaChange("Create keyspace ks2 with replication = {'class': 'SimpleStrategy', 'replication_factor': 1}"); + cluster.schemaChange("Create keyspace ks3 with replication = {'class': 'SimpleStrategy', 'replication_factor': 1}"); + } private boolean allAddressChangesEnacted(Cluster cluster) From 571df6616dbbdc14128e674ce5e0364c1c309b0c Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Tue, 17 Feb 2026 09:34:48 +0000 Subject: [PATCH 12/27] [CASSANDRA-20476] Include seeds in initial rediscovery candidates --- src/java/org/apache/cassandra/tcm/Startup.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index e8c360a20542..1d230ca1ba62 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -294,6 +294,7 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat Set candidates = new HashSet<>(previousCMS.values()); candidates.add(newAddress); + candidates.addAll(DatabaseDescriptor.getSeeds()); int maxRounds = 5; int currentRound = 0; From 1ecc2cdb5efb90b52d1fb25b6e811c60349519a3 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Wed, 18 Feb 2026 12:49:23 +0000 Subject: [PATCH 13/27] [CASSANDRA-20476] Make number of discovery rounds configurable --- src/java/org/apache/cassandra/config/Config.java | 1 + .../apache/cassandra/config/DatabaseDescriptor.java | 5 +++++ src/java/org/apache/cassandra/tcm/Discovery.java | 8 ++++---- src/java/org/apache/cassandra/tcm/Startup.java | 11 +++++------ 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 5a45073ac548..d997dc9e1487 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -1581,6 +1581,7 @@ public static void log(Config config) public volatile DurationSpec.LongMillisecondsBound progress_barrier_timeout = new DurationSpec.LongMillisecondsBound("3600000ms"); public volatile DurationSpec.LongMillisecondsBound progress_barrier_backoff = new DurationSpec.LongMillisecondsBound("1000ms"); public volatile DurationSpec.LongSecondsBound discovery_timeout = new DurationSpec.LongSecondsBound("30s"); + public volatile int discovery_rounds = 5; public boolean unsafe_tcm_mode = false; public boolean legacy_state_listener_sync_local_updates = true; diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 0885b72d2fd3..f7e21bba6a24 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -6339,6 +6339,11 @@ public static long getDiscoveryTimeout(TimeUnit unit) return conf.discovery_timeout.to(unit); } + public static int getDiscoveryRounds() + { + return conf.discovery_rounds; + } + public static boolean getUnsafeTCMMode() { return conf.unsafe_tcm_mode; diff --git a/src/java/org/apache/cassandra/tcm/Discovery.java b/src/java/org/apache/cassandra/tcm/Discovery.java index 21990118d395..a92e78cb197e 100644 --- a/src/java/org/apache/cassandra/tcm/Discovery.java +++ b/src/java/org/apache/cassandra/tcm/Discovery.java @@ -118,7 +118,7 @@ public Discovery(Supplier messaging, Supplier> surveyed = @@ -327,7 +326,7 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat // the cluster via the seeds & discovery meshing. Otherwise, if every node has a new address the non-CMS // members have no way to discover the CMS and it has no way to know the new places to push updates to. logger.info("Running discovery round; either CMS quorum was not confirmed or this is the only CMS member"); - Discovery.DiscoveredNodes nodes = Discovery.instance.discover(5, true); + Discovery.DiscoveredNodes nodes = Discovery.instance.discover(DatabaseDescriptor.getDiscoveryRounds(), true); candidates.addAll(nodes.nodes()); logger.info("Rediscovery completed, discovered nodes: {}", nodes); } From f43f1cb6d3272f3671caa4172ad5562028e2e2c5 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Wed, 18 Feb 2026 16:03:17 +0000 Subject: [PATCH 14/27] [CASSANDRA-20476] Improve comment about target quorum size during rediscovery --- src/java/org/apache/cassandra/tcm/Startup.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index dc07b0dbb300..8ee163b398c7 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -296,7 +296,10 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat candidates.add(newAddress); candidates.addAll(DatabaseDescriptor.getSeeds()); - // TODO a non-CMS node only needs to be able to contact a single CMS member to commit its STARTUP + // Technically, if this node it not a CMS member it only needs to be able to contact a single peer which is a + // CMS member to submit its STARTUP transformation. However, if this node is a CMS member, it will need to + // confirm a majority of the other members in order to join the consensus group with them in order to commit its + // own STARTUP. For simplicity we try to confirm a majority of CMS members before proceeding in either case. int quorum = (previousCMS.size() / 2) + 1; int rounds = DatabaseDescriptor.getDiscoveryRounds(); long roundTimeNanos = DatabaseDescriptor.getDiscoveryTimeout(TimeUnit.NANOSECONDS) / rounds; From 1a39507d521c7386ae822041586d01b698345cd7 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Wed, 18 Feb 2026 16:35:04 +0000 Subject: [PATCH 15/27] [CASSANDRA-20476] Simplify Discovery.state to a boolean --- .../org/apache/cassandra/tcm/Discovery.java | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/Discovery.java b/src/java/org/apache/cassandra/tcm/Discovery.java index a92e78cb197e..aaddfe444309 100644 --- a/src/java/org/apache/cassandra/tcm/Discovery.java +++ b/src/java/org/apache/cassandra/tcm/Discovery.java @@ -25,7 +25,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentSkipListSet; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import com.google.common.annotations.VisibleForTesting; @@ -90,7 +90,7 @@ public long serializedSize(NodeId t, int version) public final IVerbHandler requestHandler; private final Set discovered = new ConcurrentSkipListSet<>(); - private final AtomicReference state = new AtomicReference<>(State.NOT_STARTED); + private final AtomicBoolean inProgress = new AtomicBoolean(false); // These two have to be suppliers because during simulation we can send out discovery requests // rather early, before other node has initialized either MessagingService or DatabaseDescriptor. @@ -123,10 +123,8 @@ public DiscoveredNodes discover() public DiscoveredNodes discover(int rounds, boolean allPeers) { - boolean res = state.compareAndSet(State.NOT_STARTED, State.IN_PROGRESS); - if (!res) - res = state.compareAndSet(State.FINISHED, State.IN_PROGRESS); - assert res : String.format("Can not start discovery as it is in state %s", state.get()); + boolean res = inProgress.compareAndSet(false, true); + assert res : "Cannot start discovery as it is already running"; long discoveryTimeout = DatabaseDescriptor.getDiscoveryTimeout(TimeUnit.NANOSECONDS); long roundTimeNanos = discoveryTimeout / rounds; @@ -158,8 +156,8 @@ public DiscoveredNodes discover(int rounds, boolean allPeers) Uninterruptibles.sleepUninterruptibly(sleeptimeNanos, TimeUnit.NANOSECONDS); } - res = state.compareAndSet(State.IN_PROGRESS, State.FINISHED); - assert res : String.format("Can not finish discovery as it is in state %s", state.get()); + res = inProgress.compareAndSet(true, false); + assert res : "Cannot finish discovery as it is already complete"; return last; } @@ -305,12 +303,4 @@ public long serializedSize(DiscoveredNodes t, int version) return size; } } - - private enum State - { - NOT_STARTED, - IN_PROGRESS, - FINISHED, - FOUND_CMS - } } \ No newline at end of file From 65d7fc65bca4d7928912bc1e28a4ea52993e0ba3 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Tue, 3 Mar 2026 12:19:34 +0000 Subject: [PATCH 16/27] [CASSANDRA-20476] Include node ids in CMS description --- src/java/org/apache/cassandra/tcm/CMSOperations.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/tcm/CMSOperations.java b/src/java/org/apache/cassandra/tcm/CMSOperations.java index 60f4e67d1a6b..b6df6f8469f2 100644 --- a/src/java/org/apache/cassandra/tcm/CMSOperations.java +++ b/src/java/org/apache/cassandra/tcm/CMSOperations.java @@ -37,6 +37,7 @@ import org.apache.cassandra.db.virtual.ClusterMetadataLogTable; import org.apache.cassandra.schema.ReplicationParams; import org.apache.cassandra.schema.TableId; +import org.apache.cassandra.tcm.membership.EndpointLookup; import org.apache.cassandra.tcm.membership.NodeId; import org.apache.cassandra.tcm.membership.NodeState; import org.apache.cassandra.tcm.membership.NodeVersion; @@ -213,7 +214,12 @@ public Map describeCMS() { Map info = new HashMap<>(); ClusterMetadata metadata = ClusterMetadata.current(); - String members = metadata.fullCMSMembers().stream().sorted().map(Object::toString).collect(Collectors.joining(",")); + EndpointLookup endpoints = metadata.endpointLookup(); + String members = metadata.fullCMSMemberIds() + .stream() + .sorted() + .map(id -> String.format("(nodeid=%s,address=%s)", id.id(), endpoints.endpoint(id))) + .collect(Collectors.joining(",")); info.put(MEMBERS, members); info.put(NEEDS_RECONFIGURATION, Boolean.toString(metadata.epoch.isBefore(Epoch.FIRST) || needsReconfiguration(metadata))); info.put(IS_MEMBER, Boolean.toString(metadata.isCMSMember())); From 672e54acb8353b6c1eac16b4df2faa641b4e00df Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Fri, 3 Jul 2026 12:59:48 +0100 Subject: [PATCH 17/27] [CASSANDRA-20476] Reduce message timeout when sending Startup transform If two nodes restart with new broadcast addresses concurrently and one is a CMS member, the non-member may send a TCM_COMMIT_REQ containing its STARTUP transform to the CMS member's old address. If this is no longer reachable, the sender should time out quickly so it can resend to another CMS member, or try to discover the new CMS address(es). --- .../cassandra/tcm/ClusterMetadataService.java | 4 +++- .../org/apache/cassandra/tcm/RemoteProcessor.java | 13 +++++++------ src/java/org/apache/cassandra/tcm/Retry.java | 9 +++------ test/unit/org/apache/cassandra/tcm/RetryTest.java | 6 ++---- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java index e3f612e63405..83621d9c5630 100644 --- a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java +++ b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java @@ -33,6 +33,7 @@ import java.util.function.Function; import java.util.function.Supplier; +import com.codahale.metrics.Meter; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Sets; @@ -100,6 +101,7 @@ import static org.apache.cassandra.tcm.ClusterMetadataService.State.REMOTE; import static org.apache.cassandra.tcm.ClusterMetadataService.State.RESET; import static org.apache.cassandra.tcm.compatibility.GossipHelper.emptyWithSchemaFromSystemTables; +import static org.apache.cassandra.utils.Clock.Global.clock; import static org.apache.cassandra.utils.Clock.Global.nanoTime; import static org.apache.cassandra.utils.Collectors3.toImmutableSet; @@ -723,7 +725,7 @@ private static Retry getRetryPolicy(Transformation.Kind kind) Retry retryPolicy; if (kind == Transformation.Kind.STARTUP) { - retryPolicy = Retry.unsafeRetryIndefinitely(); + retryPolicy = Retry.withNoTimeLimit(TCMMetrics.instance.commitRetries, Retry.unsafeRetryIndefinitely()); } else if (kind == Transformation.Kind.SCHEMA_CHANGE) { diff --git a/src/java/org/apache/cassandra/tcm/RemoteProcessor.java b/src/java/org/apache/cassandra/tcm/RemoteProcessor.java index 7cfc1965ac41..a8dce93e51f8 100644 --- a/src/java/org/apache/cassandra/tcm/RemoteProcessor.java +++ b/src/java/org/apache/cassandra/tcm/RemoteProcessor.java @@ -305,8 +305,12 @@ else if (retry.hasExpired()) } InetAddressAndPort candidate = candidates.next(); - long msgExpiresAfterNanos; - if (verb == Verb.TCM_COMMIT_REQ) + long msgExpiresAfterNanos = verb.expiresAfterNanos(); + // When committing a STARTUP the deadline on the Retry will be Long.MAX_VALUE i.e. never stop, retry + // indefinitely. However, we do want to actually expire those specific messages reasonably quickly (i.e. + // in the order of rpc_timeout) so that sending one to a dead or unreachable address doesn't clog things up + // for cms_await_timeout (120s by default) and we can move on to the next candidate from the iterator + if (verb == Verb.TCM_COMMIT_REQ && retry.deadlineNanos != Long.MAX_VALUE) { long cmsAwaitNanos = DatabaseDescriptor.getCmsAwaitTimeout().to(TimeUnit.NANOSECONDS); long remainingNanos = retry.remainingNanos(); @@ -317,10 +321,7 @@ else if (retry.hasExpired()) TimeUnit.NANOSECONDS.toMillis(cmsAwaitNanos), TimeUnit.NANOSECONDS.toMillis(remainingNanos)); } - else - { - msgExpiresAfterNanos = verb.expiresAfterNanos(); - } + long msgExpiresAtNanos = MonotonicClock.Global.preciseTime.now() + msgExpiresAfterNanos; Message msg = Message.outWithFlag(verb, request, MessageFlag.CALL_BACK_ON_FAILURE, msgExpiresAtNanos); MessagingService.instance().sendWithCallback(msg, candidate, new RequestCallbackWithFailure() diff --git a/src/java/org/apache/cassandra/tcm/Retry.java b/src/java/org/apache/cassandra/tcm/Retry.java index 7b46029060bf..2e82cb8640b3 100644 --- a/src/java/org/apache/cassandra/tcm/Retry.java +++ b/src/java/org/apache/cassandra/tcm/Retry.java @@ -24,7 +24,6 @@ import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.config.DurationSpec; -import org.apache.cassandra.metrics.TCMMetrics; import org.apache.cassandra.service.RetryStrategy; import org.apache.cassandra.service.TimeoutStrategy; import org.apache.cassandra.service.TimeoutStrategy.LatencySourceFactory; @@ -37,7 +36,7 @@ public class Retry implements WaitStrategy { private static final WaitStrategy DEFAULT_STRATEGY; - private static final Retry RETRY_INDEFINITELY; + private static final WaitStrategy RETRY_INDEFINITELY; static { DurationSpec.IntMillisecondsBound defaultBackoff = DatabaseDescriptor.getDefaultRetryBackoff(); @@ -51,11 +50,9 @@ public class Retry implements WaitStrategy } DEFAULT_STRATEGY = RetryStrategy.parse(defaultSpec, LatencySourceFactory.none()); - Meter retryMeter = TCMMetrics.instance.commitRetries; String spec = (defaultBackoff == null ? "100ms" : defaultBackoff.toMilliseconds() + "ms") + "*attempts <=" + (defaultMaxBackoff == null ? "10s" : defaultMaxBackoff.toMilliseconds() + "ms"); - WaitStrategy wait = RetryStrategy.parse(spec, TimeoutStrategy.LatencySourceFactory.none()); - RETRY_INDEFINITELY = Retry.withNoTimeLimit(retryMeter, wait); + RETRY_INDEFINITELY = RetryStrategy.parse(spec, TimeoutStrategy.LatencySourceFactory.none()); } public final long deadlineNanos; @@ -74,7 +71,7 @@ public Retry(long deadlineNanos, Meter retryMeter, WaitStrategy delegate) * To be used only when submitting a STARTUP transformation when a node is restarted with a new set of addresses or * running a new release version. */ - static Retry unsafeRetryIndefinitely() + static WaitStrategy unsafeRetryIndefinitely() { return RETRY_INDEFINITELY; } diff --git a/test/unit/org/apache/cassandra/tcm/RetryTest.java b/test/unit/org/apache/cassandra/tcm/RetryTest.java index 5fe2c7ebbe28..2e401761c5b9 100644 --- a/test/unit/org/apache/cassandra/tcm/RetryTest.java +++ b/test/unit/org/apache/cassandra/tcm/RetryTest.java @@ -91,16 +91,14 @@ public long computeWait(int attempts, TimeUnit units) @Test public void testProcessorIndefiniteRetryBehaviour() { - Retry retryPolicy = Retry.unsafeRetryIndefinitely(); + WaitStrategy retryPolicy = Retry.unsafeRetryIndefinitely(); // Assert the properties of the Retry provided by the private static Processor::unsafeRetryIndefinitely for (int i = 1; i < 1000; i++) { // backoff increases in 100ms steps, up to a max of 10000ms long waitTime = retryPolicy.computeWait(i, MILLISECONDS); - assertEquals(Math.min((i + 1) * 100, 10000), waitTime); + assertEquals(Math.min((i) * 100, 10000), waitTime); } - // Retry indefinitely means no explicit deadline is set - assertEquals(Long.MAX_VALUE, retryPolicy.deadlineNanos); } @Test From 0945944fc956813a86b059fa3e129d47d591aa0a Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 9 Mar 2026 15:53:55 +0000 Subject: [PATCH 18/27] [CASSANDRA-20476] Don't attempt rediscovery if upgrading from a post CEP-21, pre V9 version --- .../org/apache/cassandra/tcm/Startup.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index 8ee163b398c7..794447d2eaf4 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -80,6 +80,7 @@ import org.apache.cassandra.tcm.sequences.InProgressSequences; import org.apache.cassandra.tcm.sequences.ReconfigureCMS; import org.apache.cassandra.tcm.sequences.ReplaceSameAddress; +import org.apache.cassandra.tcm.serialization.Version; import org.apache.cassandra.tcm.transformations.PrepareJoin; import org.apache.cassandra.tcm.transformations.PrepareReplace; import org.apache.cassandra.tcm.transformations.UnsafeJoin; @@ -136,7 +137,7 @@ public static void initialize(Set seeds, ClusterMetadata replayed = ClusterMetadata.current(); InetAddressAndPort oldAddress = replayed.directory.endpoint(nodeId); InetAddressAndPort newAddress = FBUtilities.getBroadcastAddressAndPort(); - if (!newAddress.equals(oldAddress)) + if (!newAddress.equals(oldAddress) && replayed.directory.commonSerializationVersion.isAtLeast(Version.V9)) { // Build temporary mappings for addressing CMS nodes who's addresses have been // changed but not yet committed via the CMS or not yet been enacted locally. @@ -145,6 +146,7 @@ public static void initialize(Set seeds, initializeCMSLookup(nodeId, replayed); ClusterMetadataService.instance().log().unpause(); + logger.info("Detected change in local node addresses, committing update to Cluster Metadata Service"); Transformation transform = new org.apache.cassandra.tcm.transformations.Startup(nodeId, NodeAddresses.current(), @@ -298,8 +300,8 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat // Technically, if this node it not a CMS member it only needs to be able to contact a single peer which is a // CMS member to submit its STARTUP transformation. However, if this node is a CMS member, it will need to - // confirm a majority of the other members in order to join the consensus group with them in order to commit its - // own STARTUP. For simplicity we try to confirm a majority of CMS members before proceeding in either case. + // confirm a majority of the other members in order to form the consensus group with them to commit its own + // STARTUP. For simplicity we try to confirm a majority of CMS members before proceeding in either case. int quorum = (previousCMS.size() / 2) + 1; int rounds = DatabaseDescriptor.getDiscoveryRounds(); long roundTimeNanos = DatabaseDescriptor.getDiscoveryTimeout(TimeUnit.NANOSECONDS) / rounds; @@ -308,24 +310,22 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat while (confirmedCMS.size() < quorum && currentRound < rounds) { logger.info("In round {} sending survey to {}", currentRound, candidates); - Collection> surveyed = - MessageDelivery.fanoutAndWait(MessagingService.instance(), - candidates, - Verb.TCM_DISCOVER_SURVEY_REQ, - NoPayload.noPayload, - roundTimeNanos, - TimeUnit.NANOSECONDS); + Collection> surveyed = MessageDelivery.fanoutAndWait(MessagingService.instance(), + candidates, + Verb.TCM_DISCOVER_SURVEY_REQ, + NoPayload.noPayload, + roundTimeNanos, + TimeUnit.NANOSECONDS); logger.info("Survey of {} discovered {}", candidates, surveyed); surveyed.forEach(pair -> { if (previousCMS.containsKey(pair.right)) confirmedCMS.put(pair.right, pair.left); - }); logger.info("Confirmed CMS members {}", confirmedCMS); if (confirmedCMS.size() < quorum || (previousCMS.size() == 1 && confirmedCMS.containsKey(nodeId))) { - // In the single node CMS case, run discovery simply to propagate this node's new address to the rest of + // In the single node CMS case we run discovery simply to propagate this node's new address to the rest of // the cluster via the seeds & discovery meshing. Otherwise, if every node has a new address the non-CMS // members have no way to discover the CMS and it has no way to know the new places to push updates to. logger.info("Running discovery round; either CMS quorum was not confirmed or this is the only CMS member"); From 0048124c498c2714c96e45a44d41c03445f5db9f Mon Sep 17 00:00:00 2001 From: Marcus Eriksson Date: Tue, 28 Apr 2026 14:16:32 +0200 Subject: [PATCH 19/27] [CASSANDRA-20476] Fix ip changes while still in gossip mode --- .../gms/GossipDigestAckVerbHandler.java | 2 +- .../org/apache/cassandra/gms/Gossiper.java | 13 ++- .../org/apache/cassandra/gms/NewGossiper.java | 27 +++--- .../cassandra/tcm/ClusterMetadataService.java | 2 - .../tcm/compatibility/GossipHelper.java | 40 +++++++- .../tcm/migration/GossipCMSListener.java | 10 +- ...ClusterMetadataUpgradeChangeAllIPTest.java | 34 +++++++ ...sterMetadataUpgradeChangeGossipIPTest.java | 34 +++++++ .../ClusterMetadataUpgradeChangeIPTest.java | 47 +--------- ...lusterMetadataUpgradeChangeIPTestBase.java | 91 +++++++++++++++++++ .../distributed/upgrade/UpgradeTestBase.java | 8 +- .../apache/cassandra/gms/NewGossiperTest.java | 6 +- 12 files changed, 245 insertions(+), 69 deletions(-) create mode 100644 test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeAllIPTest.java create mode 100644 test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeGossipIPTest.java create mode 100644 test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeIPTestBase.java diff --git a/src/java/org/apache/cassandra/gms/GossipDigestAckVerbHandler.java b/src/java/org/apache/cassandra/gms/GossipDigestAckVerbHandler.java index 604186a7dc8e..9796fc2fd1c8 100644 --- a/src/java/org/apache/cassandra/gms/GossipDigestAckVerbHandler.java +++ b/src/java/org/apache/cassandra/gms/GossipDigestAckVerbHandler.java @@ -56,7 +56,7 @@ public void doVerb(Message message) if (logger.isDebugEnabled()) logger.debug("Received an ack from {}, which may trigger exit from shadow round", from); - NewGossiper.instance.onAck(epStateMap); + NewGossiper.instance.onAck(message.from(), epStateMap); return; } if (epStateMap.size() > 0) diff --git a/src/java/org/apache/cassandra/gms/Gossiper.java b/src/java/org/apache/cassandra/gms/Gossiper.java index 48ef04fa07be..d0890adccfe8 100644 --- a/src/java/org/apache/cassandra/gms/Gossiper.java +++ b/src/java/org/apache/cassandra/gms/Gossiper.java @@ -720,6 +720,17 @@ public void unsafeAnnulEndpoint(InetAddressAndPort endpoint) unreachableEndpoints.remove(endpoint); } + /** + * determine which endpoint started up earlier + */ + public int compareEndpointStartup(InetAddressAndPort addr1, InetAddressAndPort addr2) + { + EndpointState ep1 = getEndpointStateForEndpoint(addr1); + EndpointState ep2 = getEndpointStateForEndpoint(addr2); + assert ep1 != null && ep2 != null; + return ep1.getHeartBeatState().getGeneration() - ep2.getHeartBeatState().getGeneration(); + } + /** * Quarantines the endpoint for QUARANTINE_DELAY * @@ -944,7 +955,7 @@ public boolean isGossipOnlyMember(InetAddressAndPort endpoint) ClusterMetadata metadata = ClusterMetadata.current(); NodeId nodeId = metadata.directory.peerId(endpoint); if (nodeId == null) - return false; + return true; return NodeState.isPreJoin(metadata.directory.states.get(nodeId)); } diff --git a/src/java/org/apache/cassandra/gms/NewGossiper.java b/src/java/org/apache/cassandra/gms/NewGossiper.java index 073943fddc33..034afb9bfff7 100644 --- a/src/java/org/apache/cassandra/gms/NewGossiper.java +++ b/src/java/org/apache/cassandra/gms/NewGossiper.java @@ -25,6 +25,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -41,7 +42,6 @@ import org.apache.cassandra.net.MessagingService; import org.apache.cassandra.tcm.ClusterMetadata; import org.apache.cassandra.tcm.compatibility.GossipHelper; -import org.apache.cassandra.utils.concurrent.Accumulator; import org.apache.cassandra.utils.concurrent.AsyncPromise; import org.apache.cassandra.utils.concurrent.Promise; @@ -60,8 +60,7 @@ public class NewGossiper public Map doShadowRound() { Set peers = new HashSet<>(SystemKeyspace.loadHostIds().keySet()); - if (peers.isEmpty()) - peers.addAll(DatabaseDescriptor.getSeeds()); + peers.addAll(DatabaseDescriptor.getSeeds()); if (peers.equals(Collections.singleton(getBroadcastAddressAndPort()))) return GossipHelper.storedEpstate(); @@ -94,18 +93,18 @@ public boolean isInShadowRound() return srh != null && !srh.isDone(); } - void onAck( Map epStateMap) + void onAck(InetAddressAndPort from, Map epStateMap) { ShadowRoundHandler srh = handler; if (srh != null && !srh.isDone()) - srh.onAck(epStateMap); + srh.onAck(from, epStateMap); } public static class ShadowRoundHandler { private volatile boolean isDone = false; private final Set peers; - private final Accumulator> responses; + private final Map> responses; private final int requiredResponses; private final MessageDelivery messageDelivery; private final Promise> promise = new AsyncPromise<>(); @@ -117,9 +116,10 @@ public ShadowRoundHandler(Set peers) public ShadowRoundHandler(Set peers, MessageDelivery messageDelivery) { - this.peers = peers; - requiredResponses = Math.max(peers.size() / 10, 1); // todo: is 10% reasonable? - responses = new Accumulator<>(requiredResponses); + this.peers = ConcurrentHashMap.newKeySet(); + this.peers.addAll(peers); + responses = new ConcurrentHashMap<>(); + requiredResponses = this.peers.size() < 3 ? 1 : Math.max(this.peers.size() / 5, 2); // require response from 20% of the cluster this.messageDelivery = messageDelivery; } @@ -146,18 +146,21 @@ public Promise> doShadowRound() return promise; } - public void onAck(Map epStateMap) + public void onAck(InetAddressAndPort from, Map epStateMap) { if (!isDone) { if (!epStateMap.isEmpty()) - responses.add(epStateMap); + { + responses.put(from, epStateMap); + peers.addAll(epStateMap.keySet()); // when retrying we should query the endpoints we learned about in the previous round + } logger.debug("Received {} responses. {} required.", responses.size(), requiredResponses); if (responses.size() >= requiredResponses) { isDone = true; - Map merged = merge(responses.snapshot()); + Map merged = merge(responses.values()); if (GossipHelper.isValidForClusterMetadata(merged)) promise.setSuccess(merged); else diff --git a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java index 83621d9c5630..b219e2bc0690 100644 --- a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java +++ b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java @@ -33,7 +33,6 @@ import java.util.function.Function; import java.util.function.Supplier; -import com.codahale.metrics.Meter; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Sets; @@ -101,7 +100,6 @@ import static org.apache.cassandra.tcm.ClusterMetadataService.State.REMOTE; import static org.apache.cassandra.tcm.ClusterMetadataService.State.RESET; import static org.apache.cassandra.tcm.compatibility.GossipHelper.emptyWithSchemaFromSystemTables; -import static org.apache.cassandra.utils.Clock.Global.clock; import static org.apache.cassandra.utils.Clock.Global.nanoTime; import static org.apache.cassandra.utils.Collectors3.toImmutableSet; diff --git a/src/java/org/apache/cassandra/tcm/compatibility/GossipHelper.java b/src/java/org/apache/cassandra/tcm/compatibility/GossipHelper.java index c60373762007..ac0a333fc7d0 100644 --- a/src/java/org/apache/cassandra/tcm/compatibility/GossipHelper.java +++ b/src/java/org/apache/cassandra/tcm/compatibility/GossipHelper.java @@ -22,6 +22,7 @@ import java.io.DataInputStream; import java.io.IOException; import java.net.UnknownHostException; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.EnumSet; @@ -33,7 +34,6 @@ import java.util.UUID; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.Lists; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -344,8 +344,41 @@ public static Map storedEpstate() return epstates; } + /** + * During upgrade, in gossip mode we allow nodes to change IP, this means that we can't decide the + * nodeId based on the sorting of the ip address, since that might change during the upgrade + * + * Instead we sort by hostId which can't change - replacements are not allowed etc. + * + * If a node has no hostId, we add the endpoints to the end, sorted by ip (if a node doesn't have a hostid it can't change ip) + * + */ + public static List sortEndpointsByHostId(Map epStates) + { + Map hostIdToEndpoint = new HashMap<>(); + List noHostId = new ArrayList<>(); + for (Map.Entry entry : epStates.entrySet()) + { + VersionedValue vv = entry.getValue().getApplicationState(HOST_ID); + if (vv == null) + noHostId.add(entry.getKey()); + else + hostIdToEndpoint.put(vv.value, entry.getKey()); + } + List sorted = new ArrayList<>(epStates.size()); + List hostIds = new ArrayList<>(hostIdToEndpoint.keySet()); + Collections.sort(hostIds); + for (String hostId : hostIds) + sorted.add(hostIdToEndpoint.get(hostId)); + Collections.sort(noHostId); + sorted.addAll(noHostId); + return sorted; + } + @VisibleForTesting - public static ClusterMetadata fromEndpointStates(Map epStates, IPartitioner partitioner, DistributedSchema schema) + public static ClusterMetadata fromEndpointStates(Map epStates, + IPartitioner partitioner, + DistributedSchema schema) { Directory directory = new Directory().withLastModified(Epoch.UPGRADE_GOSSIP); TokenMap tokenMap = new TokenMap(partitioner).withLastModified(Epoch.UPGRADE_GOSSIP); @@ -355,8 +388,7 @@ public static ClusterMetadata fromEndpointStates(Map sortedEps = Lists.newArrayList(epStates.keySet()); - Collections.sort(sortedEps); + List sortedEps = sortEndpointsByHostId(epStates); Map, ExtensionValue> extensions = new HashMap<>(); for (InetAddressAndPort endpoint : sortedEps) { diff --git a/src/java/org/apache/cassandra/tcm/migration/GossipCMSListener.java b/src/java/org/apache/cassandra/tcm/migration/GossipCMSListener.java index 8074493baed8..eec1e37dd678 100644 --- a/src/java/org/apache/cassandra/tcm/migration/GossipCMSListener.java +++ b/src/java/org/apache/cassandra/tcm/migration/GossipCMSListener.java @@ -61,8 +61,14 @@ public void onJoin(InetAddressAndPort endpoint, EndpointState epState) { UUID hostId = UUID.fromString(hostIdValue.value); nodeId = metadata.directory.nodeIdFromHostId(hostId); - logger.info("Node {} (hostId = {}) changing IP from {} to {}", nodeId, hostId, metadata.directory.endpoint(nodeId), endpoint); - Gossiper.instance.removeEndpoint(endpoint); + InetAddressAndPort oldEndpoint = metadata.directory.endpoint(nodeId); + if (Gossiper.instance.compareEndpointStartup(oldEndpoint, endpoint) > 0) + { + logger.warn("Host ID collision for {} between {} and {}; ignored {}", hostId, oldEndpoint, endpoint, endpoint); + return; + } + logger.info("Node {} (hostId = {}) changing IP from {} to {}", nodeId, hostId, oldEndpoint, endpoint); + Gossiper.instance.removeEndpoint(oldEndpoint); } else { diff --git a/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeAllIPTest.java b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeAllIPTest.java new file mode 100644 index 000000000000..82fcb3f0dafc --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeAllIPTest.java @@ -0,0 +1,34 @@ +/* + * 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.upgrade; + +import java.util.stream.IntStream; + +import org.junit.Test; + + +public class ClusterMetadataUpgradeChangeAllIPTest extends ClusterMetadataUpgradeChangeIPTestBase +{ + @Test + public void gossipModeAllIPChangeTest() throws Throwable + { + // all nodes upgraded, bouncing all nodes to new ips while in gossip mode + ipChangeTestHelper(IntStream.rangeClosed(1, NODE_COUNT).toArray()); + } +} diff --git a/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeGossipIPTest.java b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeGossipIPTest.java new file mode 100644 index 000000000000..1dc734cfed80 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeGossipIPTest.java @@ -0,0 +1,34 @@ +/* + * 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.upgrade; + +import java.util.stream.IntStream; + +import org.junit.Test; + + +public class ClusterMetadataUpgradeChangeGossipIPTest extends ClusterMetadataUpgradeChangeIPTestBase +{ + @Test + public void gossipModeIPChangeTest() throws Throwable + { + // half the nodes upgraded, bouncing all nodes to new ips (and upgrade the remaining ones) + ipChangeTestHelper(IntStream.rangeClosed(1, NODE_COUNT / 2).toArray()); + } +} diff --git a/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeIPTest.java b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeIPTest.java index c363ba5cf18a..440f71df0b15 100644 --- a/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeIPTest.java +++ b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeIPTest.java @@ -20,54 +20,13 @@ import org.junit.Test; -import org.apache.cassandra.distributed.Constants; -import org.apache.cassandra.distributed.api.Feature; -import org.apache.cassandra.distributed.api.IInstanceConfig; -import org.apache.cassandra.distributed.api.IUpgradeableInstance; -import org.apache.cassandra.distributed.api.TokenSupplier; -import org.apache.cassandra.distributed.impl.AbstractCluster; -import org.apache.cassandra.distributed.shared.NetworkTopology; -public class ClusterMetadataUpgradeChangeIPTest extends UpgradeTestBase +public class ClusterMetadataUpgradeChangeIPTest extends ClusterMetadataUpgradeChangeIPTestBase { - @Test - public void gossipModeIPChangeTest() throws Throwable - { - // all nodes upgraded, bouncing node3 to new ip while in gossip mode - ipChangeTestHelper(1, 2, 3); - } - @Test public void upgradeChangeIPTest() throws Throwable { - // changing IP while upgrading node 3 - ipChangeTestHelper(1, 2); - } - - private void ipChangeTestHelper(int ... toUpgrade) throws Throwable - { - TokenSupplier ts = TokenSupplier.evenlyDistributedTokens(3); - new TestCase() - .nodesToUpgrade(toUpgrade) - .withConfig((cfg) -> cfg.with(Feature.NETWORK, Feature.GOSSIP) - .set(Constants.KEY_DTEST_FULL_STARTUP, true)) - .withBuilder(builder -> builder.withNodeIdTopology(NetworkTopology.singleDcNetworkTopology(4, "dc0", "rack0")) - .withTokenSupplier((TokenSupplier) i -> i == 4 ? ts.tokens(3) : ts.tokens(i))) - .nodes(3) - .upgradesToCurrentFrom(v50) - .setup((cluster) -> {}) - .runAfterClusterUpgrade((cluster) -> { - cluster.get(3).shutdown().get(); - IInstanceConfig nodeConfig = cluster.newInstanceConfig(); - nodeConfig.set("data_file_directories", cluster.get(3).config().get("data_file_directories")); - IUpgradeableInstance newInstance = cluster.bootstrap(nodeConfig, AbstractCluster.CURRENT_VERSION); - newInstance.startup(); - cluster.get(1).nodetoolResult("cms", "initialize").asserts().success(); - - cluster.get(2).shutdown().get(); - cluster.get(2).startup(); - - cluster.get(2).nodetoolResult("cms", "reconfigure", "3").asserts().success(); - }).run(); + // changing all ips while upgrading + ipChangeTestHelper(); } } diff --git a/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeIPTestBase.java b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeIPTestBase.java new file mode 100644 index 000000000000..eb57a3db7c97 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadataUpgradeChangeIPTestBase.java @@ -0,0 +1,91 @@ +/* + * 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.upgrade; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Random; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.distributed.Constants; +import org.apache.cassandra.distributed.api.Feature; +import org.apache.cassandra.distributed.api.IInstanceConfig; +import org.apache.cassandra.distributed.api.IUpgradeableInstance; +import org.apache.cassandra.distributed.api.TokenSupplier; +import org.apache.cassandra.distributed.impl.AbstractCluster; +import org.apache.cassandra.distributed.shared.NetworkTopology; +import org.apache.cassandra.locator.SimpleSeedProvider; + +public class ClusterMetadataUpgradeChangeIPTestBase extends UpgradeTestBase +{ + private static final Logger logger = LoggerFactory.getLogger(ClusterMetadataUpgradeChangeIPTestBase.class); + static final int NODE_COUNT = 4; + + void ipChangeTestHelper(int ... toUpgrade) throws Throwable + { + long seed = System.currentTimeMillis(); + Random r = new Random(seed); + logger.info("SEED={}", seed); + + TokenSupplier ts = TokenSupplier.evenlyDistributedTokens(NODE_COUNT); + + new UpgradeTestBase.TestCase() + .nodesToUpgrade(toUpgrade) + .withConfig((cfg) -> cfg.with(Feature.NETWORK, Feature.GOSSIP) + .set(Constants.KEY_DTEST_FULL_STARTUP, true)) + .withBuilder(builder -> builder.withNodeIdTopology(NetworkTopology.singleDcNetworkTopology(NODE_COUNT * 2, "dc0", "rack0")) + .withTokenSupplier((TokenSupplier) i -> i > NODE_COUNT ? ts.tokens(i - NODE_COUNT) : ts.tokens(i))) + .nodes(NODE_COUNT) + .upgradesToCurrentFrom(v50) + .setup((cluster) -> {}) + .runAfterClusterUpgrade((cluster) -> { + IInstanceConfig.ParameterizedClass seedConf = + new IInstanceConfig.ParameterizedClass(SimpleSeedProvider.class.getName(), + Collections.singletonMap("seeds", "127.0.0.1,127.0.0.2,127.0.0.5,127.0.0.6,127.0.0.7")); + List ipChangeOrder = new ArrayList<>(NODE_COUNT); + for (int i = 1; i <= NODE_COUNT; i++) + { + cluster.get(i).config().set("seed_provider", seedConf); + ipChangeOrder.add(i); + } + Collections.shuffle(ipChangeOrder, r); + + for (int i : ipChangeOrder) + { + cluster.get(i).shutdown().get(); + IInstanceConfig nodeConfig = cluster.newInstanceConfig(); + nodeConfig.set("seed_provider", seedConf); + nodeConfig.set("data_file_directories", cluster.get(i).config().get("data_file_directories")); + IUpgradeableInstance newInstance = cluster.bootstrap(nodeConfig, AbstractCluster.CURRENT_VERSION); + newInstance.startup(); + } + + cluster.get(randomNode(r)).nodetoolResult("cms", "initialize").asserts().success(); + cluster.get(randomNode(r)).nodetoolResult("cms", "reconfigure", String.valueOf(NODE_COUNT)).asserts().success(); + }).run(); + } + + static int randomNode(Random r) + { + return NODE_COUNT + r.nextInt(NODE_COUNT) + 1; + } +} diff --git a/test/distributed/org/apache/cassandra/distributed/upgrade/UpgradeTestBase.java b/test/distributed/org/apache/cassandra/distributed/upgrade/UpgradeTestBase.java index 7fb41b843f43..2c60941dcece 100644 --- a/test/distributed/org/apache/cassandra/distributed/upgrade/UpgradeTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/upgrade/UpgradeTestBase.java @@ -163,6 +163,7 @@ public static class TestCase implements ThrowingRunnable private RunOnCluster runBeforeClusterUpgrade; private RunOnCluster runAfterClusterUpgrade; private final Set nodesToUpgrade = new LinkedHashSet<>(); + private boolean noNodesToUpgrade = false; private Consumer configConsumer; private Consumer builderConsumer; private TokenSupplier tokenSupplier; @@ -370,7 +371,7 @@ public void run() throws Throwable runAfterClusterUpgrade = (c) -> {}; if (runAfterNodeUpgrade == null) runAfterNodeUpgrade = (c, n) -> {}; - if (nodesToUpgrade.isEmpty()) + if (nodesToUpgrade.isEmpty() && !noNodesToUpgrade) for (int n = 1; n <= nodeCount; n++) nodesToUpgrade.add(n); @@ -416,6 +417,11 @@ public void run() throws Throwable public TestCase nodesToUpgrade(int ... nodes) { + if (nodes.length == 0) + { + noNodesToUpgrade = true; + return this; + } Set set = new HashSet<>(nodes.length); for (int n : nodes) { diff --git a/test/unit/org/apache/cassandra/gms/NewGossiperTest.java b/test/unit/org/apache/cassandra/gms/NewGossiperTest.java index a11b3d0208f2..4f15cf555ad9 100644 --- a/test/unit/org/apache/cassandra/gms/NewGossiperTest.java +++ b/test/unit/org/apache/cassandra/gms/NewGossiperTest.java @@ -70,9 +70,11 @@ public void mergeTest() throws InterruptedException, ExecutionException NewGossiper.ShadowRoundHandler srh = new NewGossiper.ShadowRoundHandler(fakePeers, new NoOpMessageDelivery()); Future> states = srh.doShadowRound(); Map firstResp = buildEpstates(fakePeers, r); - srh.onAck(firstResp); + srh.onAck(InetAddressAndPort.getByNameUnchecked("127.0.0.1"), firstResp); Map secondResp = buildEpstates(fakePeers, r); - srh.onAck(secondResp); + srh.onAck(InetAddressAndPort.getByNameUnchecked("127.0.0.2"), secondResp); + srh.onAck(InetAddressAndPort.getByNameUnchecked("127.0.0.3"), secondResp); + srh.onAck(InetAddressAndPort.getByNameUnchecked("127.0.0.4"), secondResp); Map result = states.get(); verifyResult(result, firstResp, secondResp); } From 3188c096beae69f91ec5df834a0e80fc4dcadabe Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Wed, 6 May 2026 14:03:16 +0100 Subject: [PATCH 20/27] [CASSANDRA-20476] Don't return CMS_ONLY Discovery responses when constructing a CMSLookup --- .../org/apache/cassandra/tcm/Discovery.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/tcm/Discovery.java b/src/java/org/apache/cassandra/tcm/Discovery.java index aaddfe444309..75fd83568e79 100644 --- a/src/java/org/apache/cassandra/tcm/Discovery.java +++ b/src/java/org/apache/cassandra/tcm/Discovery.java @@ -211,10 +211,32 @@ public void doVerb(Message message) { case TCM_DISCOVER_REQ: logger.trace("Responding to discovery request from {}: {}", message.from(), cms); - if (!cms.isEmpty()) + if (ClusterMetadataService.instance().log().isPaused()) + { + // The fact that the LocalLog is currently paused implies that this node is currently starting + // up and in the process of building a temporary overlay of addresses for the current CMS (see + // Startup::initialize and Startup::initializeCMSLookup). In that case, we don't want to respond + // with a definitive CMS_ONLY response containing the previous set of CMS endpoints. While these + // may still be valid, they may be outdated if the CMS member address have changed. Returning a + // CMS_ONLY response causes the requester to prioritise the returned endpoint. When those are + // outdated, this can mean that the requester must wait for retries to time out and increase the + // time taken to hit a valid CMS endpoint. + logger.info("Responding to discovery request from {}, but this node is in the process of " + + "initializing CMSLookup so not responding with a definitive CMS_ONLY response. " + + "Discovered: {}, CMS: {}", message.from(), discovered, cms); + Set allKnown = new HashSet<>(); + allKnown.addAll(discovered); + allKnown.addAll(cms); + discoveredNodes = new DiscoveredNodes(allKnown, DiscoveredNodes.Kind.KNOWN_PEERS); + } + else if (!cms.isEmpty()) + { discoveredNodes = new DiscoveredNodes(cms, DiscoveredNodes.Kind.CMS_ONLY); + } else + { discoveredNodes = new DiscoveredNodes(new HashSet<>(discovered), DiscoveredNodes.Kind.KNOWN_PEERS); + } messaging.get().send(message.responseWith(discoveredNodes), message.from()); break; case TCM_DISCOVER_PEERS_REQ: From 45af38951c1351f853877015f35cf32707005d59 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Wed, 6 May 2026 14:04:24 +0100 Subject: [PATCH 21/27] [CASSANDRA-20476] Minor logging changes --- src/java/org/apache/cassandra/tcm/CMSLookup.java | 5 +++-- src/java/org/apache/cassandra/tcm/ClusterMetadata.java | 2 +- src/java/org/apache/cassandra/tcm/Startup.java | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/CMSLookup.java b/src/java/org/apache/cassandra/tcm/CMSLookup.java index 4b50c074e243..84c592a09411 100644 --- a/src/java/org/apache/cassandra/tcm/CMSLookup.java +++ b/src/java/org/apache/cassandra/tcm/CMSLookup.java @@ -83,7 +83,7 @@ public InetAddressAndPort endpoint(NodeId id) public CMSLookup rebuild(ClusterMetadata prev, ClusterMetadata next, boolean fromSnapshot) { - logger.debug("Rebuilding CMS lookup {} with metadata from epoch {}", this, next.epoch.getEpoch()); + logger.info("Rebuilding CMS lookup {} with metadata from epoch {}", this, next.epoch.getEpoch()); // All address changes have been enacted, nothing to do if (state == State.RETIRED) @@ -126,6 +126,7 @@ public CMSLookup rebuild(ClusterMetadata prev, ClusterMetadata next, boolean fro logger.info("No changes to endpoint overrides detected"); return this; } + logger.info("Proposed endpoint overrides: {}", nextOverrides); State state = nextOverrides.isEmpty() ? State.RETIRED : State.ACTIVE; return new CMSLookup(state, next.epoch, nextOverrides); @@ -176,7 +177,7 @@ public static class LogListener implements ChangeListener @Override public void notifyPreCommit(ClusterMetadata prev, ClusterMetadata next, boolean fromSnapshot) { - logger.debug("Reevaluating CMSLookup from {} at epoch {}", prev.epoch, next.epoch); + logger.info("Reevaluating CMSLookup from {} at epoch {}", prev.epoch, next.epoch); next.refreshCMSLookup(prev, fromSnapshot); if (next.cmsLookup.state == State.RETIRED) { diff --git a/src/java/org/apache/cassandra/tcm/ClusterMetadata.java b/src/java/org/apache/cassandra/tcm/ClusterMetadata.java index 3b93bb63c7dd..5bc0fbe17598 100644 --- a/src/java/org/apache/cassandra/tcm/ClusterMetadata.java +++ b/src/java/org/apache/cassandra/tcm/ClusterMetadata.java @@ -332,7 +332,7 @@ public synchronized boolean initCMSLookup(CMSLookup lookup) { logger.info("Initializing CMS lookup on ClusterMetadata at epoch {}", epoch.getEpoch()); boolean isInitial = cmsLookup.isUninitialized(); - logger.debug("Current CMS lookup: {}, proposed: {}", cmsLookup, lookup); + logger.info("Current CMS lookup: {}, proposed: {}", cmsLookup, lookup); if (isInitial) { cmsLookup = lookup; diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index 794447d2eaf4..85630296ba47 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -161,7 +161,7 @@ public static void initialize(Set seeds, TimeUnit.MILLISECONDS.sleep(1000); // TODO make configurable? replayed = ClusterMetadata.current(); } - logger.info("Any in flight CMS address changes have been processed, current epoch is {}", replayed.epoch.getEpoch()); + logger.info("In flight CMS address changes have been processed, current epoch is {}", replayed.epoch.getEpoch()); } else { @@ -290,6 +290,7 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat if (newAddress.equals(oldAddress)) return replayed; + logger.info("Initializing CMS lookup to submit STARTUP containing broadcast address change from {} to {}", oldAddress, newAddress); Map previousCMS = new HashMap<>(); replayed.fullCMSMemberIds().forEach(id -> previousCMS.put(id, replayed.directory.endpoint(id))); Map confirmedCMS = new HashMap<>(); From 561ae8d657251ceef164d9835473c95bdd8b54d1 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 1 Jun 2026 13:09:51 +0100 Subject: [PATCH 22/27] [CASSANDRA-20476] Relocate Discovery to new package --- src/java/org/apache/cassandra/net/Verb.java | 2 +- .../org/apache/cassandra/tcm/ClusterMetadataService.java | 1 + src/java/org/apache/cassandra/tcm/RemoteProcessor.java | 2 +- src/java/org/apache/cassandra/tcm/Startup.java | 1 + .../apache/cassandra/tcm/{ => discovery}/Discovery.java | 4 +++- .../distributed/test/log/CoordinatorPathTestBase.java | 2 +- .../unit/org/apache/cassandra/tcm/RemoteProcessorTest.java | 1 + .../tcm/{ => discovery}/DiscoverySimulationTest.java | 7 ++++++- 8 files changed, 15 insertions(+), 5 deletions(-) rename src/java/org/apache/cassandra/tcm/{ => discovery}/Discovery.java (98%) rename test/unit/org/apache/cassandra/tcm/{ => discovery}/DiscoverySimulationTest.java (96%) diff --git a/src/java/org/apache/cassandra/net/Verb.java b/src/java/org/apache/cassandra/net/Verb.java index d3b1adab1879..404731c6cd16 100644 --- a/src/java/org/apache/cassandra/net/Verb.java +++ b/src/java/org/apache/cassandra/net/Verb.java @@ -136,10 +136,10 @@ import org.apache.cassandra.streaming.DataMovement; import org.apache.cassandra.streaming.DataMovementVerbHandler; import org.apache.cassandra.streaming.ReplicationDoneVerbHandler; -import org.apache.cassandra.tcm.Discovery; import org.apache.cassandra.tcm.Epoch; import org.apache.cassandra.tcm.FetchCMSLog; import org.apache.cassandra.tcm.FetchPeerLog; +import org.apache.cassandra.tcm.discovery.Discovery; import org.apache.cassandra.tcm.migration.CMSInitializationRequest; import org.apache.cassandra.tcm.migration.CMSInitializationResponse; import org.apache.cassandra.tcm.migration.Election; diff --git a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java index b219e2bc0690..c16c2550bb28 100644 --- a/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java +++ b/src/java/org/apache/cassandra/tcm/ClusterMetadataService.java @@ -61,6 +61,7 @@ import org.apache.cassandra.service.accord.topology.AccordFastPath; import org.apache.cassandra.service.accord.topology.AccordStaleReplicas; import org.apache.cassandra.service.consensus.migration.ConsensusMigrationState; +import org.apache.cassandra.tcm.discovery.Discovery; import org.apache.cassandra.tcm.listeners.SchemaListener; import org.apache.cassandra.tcm.log.Entry; import org.apache.cassandra.tcm.log.LocalLog; diff --git a/src/java/org/apache/cassandra/tcm/RemoteProcessor.java b/src/java/org/apache/cassandra/tcm/RemoteProcessor.java index a8dce93e51f8..f381821eb86e 100644 --- a/src/java/org/apache/cassandra/tcm/RemoteProcessor.java +++ b/src/java/org/apache/cassandra/tcm/RemoteProcessor.java @@ -51,7 +51,7 @@ import org.apache.cassandra.net.MessagingService; import org.apache.cassandra.net.RequestCallbackWithFailure; import org.apache.cassandra.net.Verb; -import org.apache.cassandra.tcm.Discovery.DiscoveredNodes; +import org.apache.cassandra.tcm.discovery.Discovery.DiscoveredNodes; import org.apache.cassandra.tcm.log.Entry; import org.apache.cassandra.tcm.log.LocalLog; import org.apache.cassandra.tcm.log.LogState; diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index 85630296ba47..4b03aa4db4ca 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -65,6 +65,7 @@ import org.apache.cassandra.schema.SchemaKeyspace; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.service.StorageService; +import org.apache.cassandra.tcm.discovery.Discovery; import org.apache.cassandra.tcm.log.LocalLog; import org.apache.cassandra.tcm.log.LogStorage; import org.apache.cassandra.tcm.log.SystemKeyspaceStorage; diff --git a/src/java/org/apache/cassandra/tcm/Discovery.java b/src/java/org/apache/cassandra/tcm/discovery/Discovery.java similarity index 98% rename from src/java/org/apache/cassandra/tcm/Discovery.java rename to src/java/org/apache/cassandra/tcm/discovery/Discovery.java index 75fd83568e79..fd3c999f7355 100644 --- a/src/java/org/apache/cassandra/tcm/Discovery.java +++ b/src/java/org/apache/cassandra/tcm/discovery/Discovery.java @@ -16,7 +16,7 @@ * limitations under the License. */ -package org.apache.cassandra.tcm; +package org.apache.cassandra.tcm.discovery; import java.io.IOException; import java.util.ArrayList; @@ -48,6 +48,8 @@ import org.apache.cassandra.net.MessagingService; import org.apache.cassandra.net.NoPayload; import org.apache.cassandra.net.Verb; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.tcm.ClusterMetadataService; import org.apache.cassandra.tcm.membership.NodeId; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.Pair; diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/CoordinatorPathTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/log/CoordinatorPathTestBase.java index 28125692acf2..17f247b9b33d 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/log/CoordinatorPathTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/log/CoordinatorPathTestBase.java @@ -79,13 +79,13 @@ import org.apache.cassandra.tcm.ClusterMetadata; import org.apache.cassandra.tcm.ClusterMetadataService; import org.apache.cassandra.tcm.Commit; -import org.apache.cassandra.tcm.Discovery; import org.apache.cassandra.tcm.Epoch; import org.apache.cassandra.tcm.FetchCMSLog; import org.apache.cassandra.tcm.MetadataSnapshots; import org.apache.cassandra.tcm.Processor; import org.apache.cassandra.tcm.Retry; import org.apache.cassandra.tcm.Transformation; +import org.apache.cassandra.tcm.discovery.Discovery; import org.apache.cassandra.tcm.log.Entry; import org.apache.cassandra.tcm.log.LocalLog; import org.apache.cassandra.tcm.log.LogState; diff --git a/test/unit/org/apache/cassandra/tcm/RemoteProcessorTest.java b/test/unit/org/apache/cassandra/tcm/RemoteProcessorTest.java index 85dcfac8a1bd..285f8c1ff853 100644 --- a/test/unit/org/apache/cassandra/tcm/RemoteProcessorTest.java +++ b/test/unit/org/apache/cassandra/tcm/RemoteProcessorTest.java @@ -31,6 +31,7 @@ import org.apache.cassandra.config.Config; import org.apache.cassandra.locator.InetAddressAndPort; import org.apache.cassandra.locator.Locator; +import org.apache.cassandra.tcm.discovery.Discovery; import org.apache.cassandra.tcm.membership.Location; import static org.junit.Assert.assertEquals; diff --git a/test/unit/org/apache/cassandra/tcm/DiscoverySimulationTest.java b/test/unit/org/apache/cassandra/tcm/discovery/DiscoverySimulationTest.java similarity index 96% rename from test/unit/org/apache/cassandra/tcm/DiscoverySimulationTest.java rename to test/unit/org/apache/cassandra/tcm/discovery/DiscoverySimulationTest.java index 5f5024c7aef5..17b579b3c4fa 100644 --- a/test/unit/org/apache/cassandra/tcm/DiscoverySimulationTest.java +++ b/test/unit/org/apache/cassandra/tcm/discovery/DiscoverySimulationTest.java @@ -16,7 +16,7 @@ * limitations under the License. */ -package org.apache.cassandra.tcm; +package org.apache.cassandra.tcm.discovery; import java.io.IOException; import java.util.Collections; @@ -44,6 +44,11 @@ import org.apache.cassandra.net.MessageDelivery; import org.apache.cassandra.net.RequestCallback; import org.apache.cassandra.net.Verb; +import org.apache.cassandra.tcm.AtomicLongBackedProcessor; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.tcm.ClusterMetadataService; +import org.apache.cassandra.tcm.Commit; +import org.apache.cassandra.tcm.MetadataSnapshots; import org.apache.cassandra.tcm.log.LocalLog; import org.apache.cassandra.tcm.ownership.UniformRangePlacement; import org.apache.cassandra.utils.concurrent.Future; From 42fc3ee8c897cee479a8e503a904577945e8393a Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 8 Jun 2026 12:27:21 +0100 Subject: [PATCH 23/27] [CASSANDRA-20476] Include ClusterMetadata.metadataId in survey req/rsp messages --- .../apache/cassandra/net/MessageDelivery.java | 1 + src/java/org/apache/cassandra/net/Verb.java | 7 +- .../org/apache/cassandra/tcm/Startup.java | 30 +++-- .../cassandra/tcm/discovery/Discovery.java | 39 ++----- .../tcm/discovery/SurveyRequest.java | 67 +++++++++++ .../tcm/discovery/SurveyRequestHandler.java | 85 ++++++++++++++ .../tcm/discovery/SurveyResponse.java | 75 +++++++++++++ .../discovery/SurveyRequestHandlerTest.java | 106 ++++++++++++++++++ 8 files changed, 367 insertions(+), 43 deletions(-) create mode 100644 src/java/org/apache/cassandra/tcm/discovery/SurveyRequest.java create mode 100644 src/java/org/apache/cassandra/tcm/discovery/SurveyRequestHandler.java create mode 100644 src/java/org/apache/cassandra/tcm/discovery/SurveyResponse.java create mode 100644 test/unit/org/apache/cassandra/tcm/discovery/SurveyRequestHandlerTest.java diff --git a/src/java/org/apache/cassandra/net/MessageDelivery.java b/src/java/org/apache/cassandra/net/MessageDelivery.java index 064dacb0e050..133fdc26de60 100644 --- a/src/java/org/apache/cassandra/net/MessageDelivery.java +++ b/src/java/org/apache/cassandra/net/MessageDelivery.java @@ -53,6 +53,7 @@ static Collection> fanoutAndWait(Messag { return fanoutAndWait(messaging, sendTo, verb, payload, DatabaseDescriptor.getCmsAwaitTimeout().to(TimeUnit.MILLISECONDS), TimeUnit.MILLISECONDS); } + static Collection> fanoutAndWait(MessageDelivery messaging, Set sendTo, Verb verb, REQ payload, long timeout, TimeUnit timeUnit) { Accumulator> responses = new Accumulator<>(sendTo.size()); diff --git a/src/java/org/apache/cassandra/net/Verb.java b/src/java/org/apache/cassandra/net/Verb.java index 404731c6cd16..46c1d843593f 100644 --- a/src/java/org/apache/cassandra/net/Verb.java +++ b/src/java/org/apache/cassandra/net/Verb.java @@ -140,6 +140,9 @@ import org.apache.cassandra.tcm.FetchCMSLog; import org.apache.cassandra.tcm.FetchPeerLog; import org.apache.cassandra.tcm.discovery.Discovery; +import org.apache.cassandra.tcm.discovery.SurveyRequest; +import org.apache.cassandra.tcm.discovery.SurveyRequestHandler; +import org.apache.cassandra.tcm.discovery.SurveyResponse; import org.apache.cassandra.tcm.migration.CMSInitializationRequest; import org.apache.cassandra.tcm.migration.CMSInitializationResponse; import org.apache.cassandra.tcm.migration.Election; @@ -318,8 +321,8 @@ public enum Verb TCM_FETCH_PEER_LOG_REQ (819, P0, rpcTimeout, FETCH_METADATA, () -> FetchPeerLog.serializer, () -> FetchPeerLog.Handler.instance, TCM_FETCH_PEER_LOG_RSP ), TCM_DISCOVER_PEERS_RSP (820, P0, rpcTimeout, INTERNAL_METADATA, () -> Discovery.serializer, () -> ResponseVerbHandler.instance ), TCM_DISCOVER_PEERS_REQ (821, P0, rpcTimeout, INTERNAL_METADATA, () -> NoPayload.serializer, () -> Discovery.instance.requestHandler, TCM_DISCOVER_PEERS_RSP), - TCM_DISCOVER_SURVEY_RSP(822, P0, rpcTimeout, INTERNAL_METADATA, () -> Discovery.nodeIdSerializer, () -> ResponseVerbHandler.instance ), - TCM_DISCOVER_SURVEY_REQ(823, P0, rpcTimeout, INTERNAL_METADATA, () -> NoPayload.serializer, () -> Discovery.instance.requestHandler, TCM_DISCOVER_SURVEY_RSP), + TCM_DISCOVER_SURVEY_RSP(822, P0, rpcTimeout, INTERNAL_METADATA, () -> SurveyResponse.serializer, () -> ResponseVerbHandler.instance ), + TCM_DISCOVER_SURVEY_REQ(823, P0, rpcTimeout, INTERNAL_METADATA, () -> SurveyRequest.serializer, () -> SurveyRequestHandler.instance(), TCM_DISCOVER_SURVEY_RSP), INITIATE_DATA_MOVEMENTS_RSP (814, P1, rpcTimeout, MISC, () -> NoPayload.serializer, RESPONSE_HANDLER ), INITIATE_DATA_MOVEMENTS_REQ (815, P1, rpcTimeout, MISC, () -> DataMovement.serializer, () -> DataMovementVerbHandler.instance, INITIATE_DATA_MOVEMENTS_RSP ), diff --git a/src/java/org/apache/cassandra/tcm/Startup.java b/src/java/org/apache/cassandra/tcm/Startup.java index 4b03aa4db4ca..fb7e02ba69a4 100644 --- a/src/java/org/apache/cassandra/tcm/Startup.java +++ b/src/java/org/apache/cassandra/tcm/Startup.java @@ -56,7 +56,6 @@ import org.apache.cassandra.locator.InetAddressAndPort; import org.apache.cassandra.net.MessageDelivery; import org.apache.cassandra.net.MessagingService; -import org.apache.cassandra.net.NoPayload; import org.apache.cassandra.net.Verb; import org.apache.cassandra.schema.DistributedSchema; import org.apache.cassandra.schema.KeyspaceMetadata; @@ -66,6 +65,8 @@ import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.service.StorageService; import org.apache.cassandra.tcm.discovery.Discovery; +import org.apache.cassandra.tcm.discovery.SurveyRequest; +import org.apache.cassandra.tcm.discovery.SurveyResponse; import org.apache.cassandra.tcm.log.LocalLog; import org.apache.cassandra.tcm.log.LogStorage; import org.apache.cassandra.tcm.log.SystemKeyspaceStorage; @@ -159,7 +160,7 @@ public static void initialize(Set seeds, while (replayed.cmsLookup.isActive()) { logger.info("Waiting for pending CMS address changes to complete {}", replayed.cmsLookup); - TimeUnit.MILLISECONDS.sleep(1000); // TODO make configurable? + TimeUnit.MILLISECONDS.sleep(1000); replayed = ClusterMetadata.current(); } logger.info("In flight CMS address changes have been processed, current epoch is {}", replayed.epoch.getEpoch()); @@ -312,16 +313,25 @@ private static ClusterMetadata initializeCMSLookup(NodeId nodeId, ClusterMetadat while (confirmedCMS.size() < quorum && currentRound < rounds) { logger.info("In round {} sending survey to {}", currentRound, candidates); - Collection> surveyed = MessageDelivery.fanoutAndWait(MessagingService.instance(), - candidates, - Verb.TCM_DISCOVER_SURVEY_REQ, - NoPayload.noPayload, - roundTimeNanos, - TimeUnit.NANOSECONDS); + SurveyRequest request = new SurveyRequest(replayed.metadataIdentifier); + Collection> surveyed = MessageDelivery.fanoutAndWait(MessagingService.instance(), + candidates, + Verb.TCM_DISCOVER_SURVEY_REQ, + request, + roundTimeNanos, + TimeUnit.NANOSECONDS); logger.info("Survey of {} discovered {}", candidates, surveyed); surveyed.forEach(pair -> { - if (previousCMS.containsKey(pair.right)) - confirmedCMS.put(pair.right, pair.left); + SurveyResponse response = pair.right; + if (response.metadataId == replayed.metadataIdentifier) + { + if (previousCMS.containsKey(response.nodeId)) + confirmedCMS.put(response.nodeId, pair.left); + } + else + { + logger.info("Mismatching metadata id in survey response from {}, ignoring ({}/{})", pair.left, replayed.metadataIdentifier, response.metadataId); + } }); logger.info("Confirmed CMS members {}", confirmedCMS); diff --git a/src/java/org/apache/cassandra/tcm/discovery/Discovery.java b/src/java/org/apache/cassandra/tcm/discovery/Discovery.java index fd3c999f7355..9e2e781d696a 100644 --- a/src/java/org/apache/cassandra/tcm/discovery/Discovery.java +++ b/src/java/org/apache/cassandra/tcm/discovery/Discovery.java @@ -36,7 +36,6 @@ import org.slf4j.LoggerFactory; import org.apache.cassandra.config.DatabaseDescriptor; -import org.apache.cassandra.db.SystemKeyspace; import org.apache.cassandra.db.TypeSizes; import org.apache.cassandra.io.IVersionedSerializer; import org.apache.cassandra.io.util.DataInputPlus; @@ -50,7 +49,6 @@ import org.apache.cassandra.net.Verb; import org.apache.cassandra.tcm.ClusterMetadata; import org.apache.cassandra.tcm.ClusterMetadataService; -import org.apache.cassandra.tcm.membership.NodeId; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.Pair; @@ -67,28 +65,6 @@ public class Discovery public static final Discovery instance = new Discovery(); public static final Serializer serializer = new Serializer(); - // TODO add this to MessageSerializers properly or define a real response format - public static final IVersionedSerializer nodeIdSerializer = new IVersionedSerializer<>() - { - @Override - public void serialize(NodeId t, DataOutputPlus out, int version) throws IOException - { - out.writeUnsignedVInt32(t.id()); - } - - @Override - public NodeId deserialize(DataInputPlus in, int version) throws IOException - { - int id = in.readUnsignedVInt32(); - return new NodeId(id); - } - - @Override - public long serializedSize(NodeId t, int version) - { - return TypeSizes.sizeofUnsignedVInt(t.id()); - } - }; public final IVerbHandler requestHandler; private final Set discovered = new ConcurrentSkipListSet<>(); @@ -194,6 +170,11 @@ public DiscoveredNodes discoverOnce(boolean allPeers, InetAddressAndPort initiat return new DiscoveredNodes(discovered, DiscoveredNodes.Kind.KNOWN_PEERS); } + public void discovered(InetAddressAndPort peer) + { + discovered.add(peer); + } + private final class DiscoveryRequestHandler implements IVerbHandler { final Supplier messaging; @@ -207,7 +188,8 @@ private final class DiscoveryRequestHandler implements IVerbHandler public void doVerb(Message message) { discovered.add(message.from()); - Set cms = ClusterMetadata.current().fullCMSMembers(); + ClusterMetadata metadata = ClusterMetadata.current(); + Set cms = metadata.fullCMSMembers(); DiscoveredNodes discoveredNodes; switch (message.verb()) { @@ -242,15 +224,10 @@ else if (!cms.isEmpty()) messaging.get().send(message.responseWith(discoveredNodes), message.from()); break; case TCM_DISCOVER_PEERS_REQ: - logger.trace("Responding to {} request from {}", message.verb(), message.from()); + logger.info("Responding to {} request from {}", message.verb(), message.from()); discoveredNodes = new DiscoveredNodes(new HashSet<>(discovered), DiscoveredNodes.Kind.KNOWN_PEERS); messaging.get().send(message.responseWith(discoveredNodes), message.from()); break; - case TCM_DISCOVER_SURVEY_REQ: - logger.trace("Responding to {} request from {}", message.verb(), message.from()); - NodeId id = NodeId.fromUUID(SystemKeyspace.getLocalHostId()); - messaging.get().send(message.responseWith(id), message.from()); - break; } } } diff --git a/src/java/org/apache/cassandra/tcm/discovery/SurveyRequest.java b/src/java/org/apache/cassandra/tcm/discovery/SurveyRequest.java new file mode 100644 index 000000000000..b774298b6664 --- /dev/null +++ b/src/java/org/apache/cassandra/tcm/discovery/SurveyRequest.java @@ -0,0 +1,67 @@ +/* + * 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.tcm.discovery; + +import java.io.IOException; + +import org.apache.cassandra.db.TypeSizes; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; + +public class SurveyRequest +{ + public static final Serializer serializer = new Serializer(); + final int metadataId; + + public SurveyRequest(int metadataId) + { + this.metadataId = metadataId; + } + + @Override + public String toString() + { + return "SurveyRequest{" + + "metadataId=" + metadataId + + '}'; + } + + public static class Serializer implements IVersionedSerializer + { + @Override + public void serialize(SurveyRequest t, DataOutputPlus out, int version) throws IOException + { + out.writeUnsignedVInt32(t.metadataId); + } + + @Override + public SurveyRequest deserialize(DataInputPlus in, int version) throws IOException + { + int metadataId = in.readUnsignedVInt32(); + return new SurveyRequest(metadataId); + } + + @Override + public long serializedSize(SurveyRequest t, int version) + { + return TypeSizes.sizeofUnsignedVInt(t.metadataId); + } + } +} diff --git a/src/java/org/apache/cassandra/tcm/discovery/SurveyRequestHandler.java b/src/java/org/apache/cassandra/tcm/discovery/SurveyRequestHandler.java new file mode 100644 index 000000000000..aaa9ffa2d1eb --- /dev/null +++ b/src/java/org/apache/cassandra/tcm/discovery/SurveyRequestHandler.java @@ -0,0 +1,85 @@ +/* + * 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.tcm.discovery; + +import java.io.IOException; +import java.util.function.Supplier; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.db.SystemKeyspace; +import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.net.IVerbHandler; +import org.apache.cassandra.net.Message; +import org.apache.cassandra.net.MessageDelivery; +import org.apache.cassandra.net.MessagingService; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.tcm.membership.NodeId; + +public class SurveyRequestHandler implements IVerbHandler +{ + private static final Logger logger = LoggerFactory.getLogger(SurveyRequestHandler.class); + private static volatile SurveyRequestHandler instance; + + final Supplier messaging; + public final int metadataId; + + public static SurveyRequestHandler instance() + { + if (instance == null) + { + synchronized (SurveyRequestHandler.class) + { + if (instance == null) + instance = new SurveyRequestHandler(); + } + } + return instance; + } + + private SurveyRequestHandler() + { + this(ClusterMetadata.current().metadataIdentifier, MessagingService::instance); + } + + public SurveyRequestHandler(int metadataId, Supplier messaging) + { + this.metadataId = metadataId; + this.messaging = messaging; + } + + @Override + public void doVerb(Message message) throws IOException + { + logger.info("Responding to {} request from {}", message.verb(), message.from()); + if (message.payload.metadataId != metadataId) + throw new InvalidRequestException(String.format("Mismatching metadata id in survey request from %s (%d)", + message.from(), + message.payload.metadataId)); + + Discovery.instance.discovered(message.from()); + // Respond with the node id from system.local and not ClusterMetadata.current().myNodeId() because if + // this node is in the process of starting up with a new broadcast address, it will not yet recognise itself + // as being in a REGISTERED state. This results in myNodeId() returning NodeId.UNREGISTERED. + NodeId nodeId = NodeId.fromUUID(SystemKeyspace.getLocalHostId()); + SurveyResponse response = new SurveyResponse(metadataId, nodeId); + messaging.get().respond(response, message); + } +} diff --git a/src/java/org/apache/cassandra/tcm/discovery/SurveyResponse.java b/src/java/org/apache/cassandra/tcm/discovery/SurveyResponse.java new file mode 100644 index 000000000000..83819658b6ac --- /dev/null +++ b/src/java/org/apache/cassandra/tcm/discovery/SurveyResponse.java @@ -0,0 +1,75 @@ +/* + * 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.tcm.discovery; + +import java.io.IOException; + +import org.apache.cassandra.db.TypeSizes; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.tcm.membership.NodeId; + +public class SurveyResponse +{ + public static final Serializer serializer = new Serializer(); + public final int metadataId; + public final NodeId nodeId; + + public SurveyResponse(int metadataId, NodeId nodeId) + { + this.metadataId = metadataId; + this.nodeId = nodeId; + } + + @Override + public String toString() + { + return "SurveyResponse{" + + "metadataId=" + metadataId + + ", nodeId=" + nodeId + + '}'; + } + + public static class Serializer implements IVersionedSerializer + { + @Override + public void serialize(SurveyResponse t, DataOutputPlus out, int version) throws IOException + { + out.writeUnsignedVInt32(t.metadataId); + out.writeUnsignedVInt32(t.nodeId.id()); + } + + @Override + public SurveyResponse deserialize(DataInputPlus in, int version) throws IOException + { + int metadataId = in.readUnsignedVInt32(); + int nodeId = in.readUnsignedVInt32(); + return new SurveyResponse(metadataId, new NodeId(nodeId)); + } + + @Override + public long serializedSize(SurveyResponse t, int version) + { + return TypeSizes.sizeofUnsignedVInt(t.metadataId) + TypeSizes.sizeofUnsignedVInt(t.nodeId.id()); + } + } + + ; +} diff --git a/test/unit/org/apache/cassandra/tcm/discovery/SurveyRequestHandlerTest.java b/test/unit/org/apache/cassandra/tcm/discovery/SurveyRequestHandlerTest.java new file mode 100644 index 000000000000..50a2da973abe --- /dev/null +++ b/test/unit/org/apache/cassandra/tcm/discovery/SurveyRequestHandlerTest.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.tcm.discovery; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.ServerTestUtils; +import org.apache.cassandra.db.SystemKeyspace; +import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.locator.InetAddressAndPort; +import org.apache.cassandra.net.ConnectionType; +import org.apache.cassandra.net.Message; +import org.apache.cassandra.net.MessageDelivery; +import org.apache.cassandra.net.RequestCallback; +import org.apache.cassandra.net.Verb; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.tcm.membership.NodeId; +import org.apache.cassandra.utils.concurrent.Future; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +public class SurveyRequestHandlerTest +{ + @BeforeClass + public static void initClusterMetadata() + { + ServerTestUtils.prepareServerNoRegister(); + } + + @Test + public void testRequestWithMismatchingMetadataIdIsRejected() throws IOException + { + StubMessageDelivery messaging = new StubMessageDelivery(); + SurveyRequestHandler handler = new SurveyRequestHandler(999, () -> messaging); + try + { + handler.doVerb(Message.out(Verb.TCM_DISCOVER_SURVEY_REQ, new SurveyRequest(0))); + fail("Expected InvalidRequestException"); + } + catch (InvalidRequestException e) + { + assertEquals("Mismatching metadata id in survey request from /127.0.0.1:7012 (0)",e.getMessage()); + } + } + + @Test + public void testRespondWithNodeIdFromSystemTable() throws IOException + { + ClusterMetadata metadata = ClusterMetadata.current(); + StubMessageDelivery messaging = new StubMessageDelivery(); + SurveyRequestHandler handler = new SurveyRequestHandler(metadata.metadataIdentifier, () -> messaging); + assertEquals(NodeId.UNREGISTERED, ClusterMetadata.current().myNodeId()); + NodeId id = new NodeId(555); + SystemKeyspace.setLocalHostId(id.toUUID()); + handler.doVerb(Message.out(Verb.TCM_DISCOVER_SURVEY_REQ, new SurveyRequest(0))); + assertEquals(1, messaging.responses.size()); + SurveyResponse response = (SurveyResponse) messaging.responses.get(0); + assertEquals(id, response.nodeId); + assertEquals(metadata.metadataIdentifier, response.metadataId); + } + + private static class StubMessageDelivery implements MessageDelivery + { + + List responses = new ArrayList<>(); + @Override + public void respond(V response, Message message) + { + responses.add(response); + } + + @Override + public void send(Message message, InetAddressAndPort to) {} + + @Override + public void sendWithCallback(Message message, InetAddressAndPort to, RequestCallback cb) {} + + @Override + public void sendWithCallback(Message message, InetAddressAndPort to, RequestCallback cb, ConnectionType specifyConnection) {} + + @Override + public Future> sendWithResult(Message message, InetAddressAndPort to) {return null;} + } +} From fc802b7180f0135cb204fcac4b606e9f6c1773e1 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 8 Jun 2026 12:26:32 +0100 Subject: [PATCH 24/27] [CASSANDRA-20476] Lower ProgressBarrier backoff in DiscoverNewCMSTest --- .../distributed/test/log/DiscoverNewCMSTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java b/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java index 7865540ed9a6..d2bdbd31b354 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java @@ -70,7 +70,8 @@ public void disableAccord() public void singleNodeCMSAddressChangeTest() throws IOException, ExecutionException, InterruptedException { try (Cluster cluster = builder().withNodes(3) - .withConfig(config -> config.with(NETWORK, GOSSIP)) + .withConfig(config -> config.with(NETWORK, GOSSIP) + .set("progress_barrier_backoff", "50ms")) .createWithoutStarting()) { test(cluster, 1); @@ -82,7 +83,8 @@ public void multiNodeCMSOnlyClusterAddressChangeTest() throws IOException, Execu { try (Cluster cluster = builder().withNodes(3) - .withConfig(config -> config.with(NETWORK, GOSSIP)) + .withConfig(config -> config.with(NETWORK, GOSSIP) + .set("progress_barrier_backoff", "50ms")) .createWithoutStarting()) { test(cluster, 3); @@ -93,7 +95,8 @@ public void multiNodeCMSOnlyClusterAddressChangeTest() throws IOException, Execu public void multiNodeCMSAllAddressesChangeTest() throws IOException, ExecutionException, InterruptedException { try (Cluster cluster = builder().withNodes(6) - .withConfig(config -> config.with(NETWORK, GOSSIP)) + .withConfig(config -> config.with(NETWORK, GOSSIP) + .set("progress_barrier_backoff", "50ms")) .createWithoutStarting()) { test(cluster, 3); From 718e4a4c53c9d7b06934221cacd10fb927331904 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Thu, 16 Jul 2026 18:13:47 +0100 Subject: [PATCH 25/27] [CASSANDRA-20476] Improvements to RemoteProcessor.CandidateIterator --- .../apache/cassandra/tcm/RemoteProcessor.java | 30 ++++++++++++++----- .../cassandra/tcm/RemoteProcessorTest.java | 24 --------------- 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/java/org/apache/cassandra/tcm/RemoteProcessor.java b/src/java/org/apache/cassandra/tcm/RemoteProcessor.java index f381821eb86e..fa718de36565 100644 --- a/src/java/org/apache/cassandra/tcm/RemoteProcessor.java +++ b/src/java/org/apache/cassandra/tcm/RemoteProcessor.java @@ -22,7 +22,9 @@ import java.util.Collection; import java.util.Collections; import java.util.Deque; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -419,6 +421,7 @@ public void onFailure(InetAddressAndPort from, RequestFailure failureReason) public static class CandidateIterator extends AbstractIterator { private final Deque candidates; + private final Set elements; private final boolean checkLive; @SuppressWarnings("resource") @@ -431,6 +434,7 @@ public CandidateIterator(Collection initialContacts) public CandidateIterator(Collection initialContacts, boolean checkLive) { this.candidates = new ConcurrentLinkedDeque<>(initialContacts); + this.elements = new HashSet<>(initialContacts); this.checkLive = checkLive; } @@ -442,19 +446,26 @@ public CandidateIterator(Collection initialContacts, boolean public void addCandidates(DiscoveredNodes discoveredNodes) { if (discoveredNodes.kind() == DiscoveredNodes.Kind.CMS_ONLY) - discoveredNodes.nodes().forEach(candidates::addFirst); + discoveredNodes.nodes().forEach(this::maybeAddFirst); else - discoveredNodes.nodes().forEach(candidates::addLast); + discoveredNodes.nodes().forEach(this::maybeAddLast); } - public void notCms(InetAddressAndPort resp) + private void maybeAddFirst(InetAddressAndPort candidate) { - candidates.addLast(resp); + if (elements.add(candidate)) + candidates.addFirst(candidate); + } + + private void maybeAddLast(InetAddressAndPort candidate) + { + if (elements.add(candidate)) + candidates.addLast(candidate); } public void timeout(InetAddressAndPort timedOut) { - candidates.addLast(timedOut); + maybeAddLast(timedOut); } public String toString() @@ -488,14 +499,17 @@ else if (first.equals(ep)) if (checkLive && !FailureDetector.instance.isAlive(ep)) { - if (candidates.isEmpty()) - return ep; - else + // If there are other candidates, just return this one to the back of the deque. It can be added + // directly, not via maybeAddLast as it hasn't been removed from the element set yet + if (!candidates.isEmpty()) { candidates.addLast(ep); continue; } } + // if we have a candidate, it was popped from the deque so make sure it's also removed from the set + if (ep != null) + elements.remove(ep); return ep; } return endOfData(); diff --git a/test/unit/org/apache/cassandra/tcm/RemoteProcessorTest.java b/test/unit/org/apache/cassandra/tcm/RemoteProcessorTest.java index 285f8c1ff853..b06d69c94ac1 100644 --- a/test/unit/org/apache/cassandra/tcm/RemoteProcessorTest.java +++ b/test/unit/org/apache/cassandra/tcm/RemoteProcessorTest.java @@ -80,30 +80,6 @@ public void timeoutTest() } } - @Test - public void notCMSTest() - { - // make sure that a node marked as notCMS will not be returned until we've cycled through all other candidates - // when using the iterator in a RemoteProcessor::sendWithCallback call, the Backoff will trigger the breaking - // out of the cycle. - int endpointCount = 10; - List allEndpoints = eps(endpointCount); - Set discovery = new HashSet<>(allEndpoints.subList(0, 4)); - RemoteProcessor.CandidateIterator iter = new RemoteProcessor.CandidateIterator(discovery, false); - InetAddressAndPort notcms = iter.peek(); - for (int i = 1; i < 10; i++) - { - assertTrue(iter.hasNext()); - InetAddressAndPort returned = iter.next(); - assertTrue(discovery.contains(returned)); - if (returned.equals(notcms)) - { - iter.notCms(returned); - assertEquals(notcms, iter.peekLast()); - } - } - } - private List eps(int endpointCount) { List allEndpoints = new ArrayList<>(endpointCount); From 1cafe667b26aa2917348de0abaf9074a5c89c653 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Tue, 21 Jul 2026 16:42:51 +0100 Subject: [PATCH 26/27] [CASSANDRA-20476] Break up new long running test --- .../log/AllClusterCMSRediscoveryTest.java | 38 +++ .../test/log/CMSRediscoveryTestBase.java | 224 ++++++++++++++++ .../test/log/DiscoverNewCMSTest.java | 244 ------------------ .../test/log/MultiNodeCMSRediscoveryTest.java | 38 +++ .../log/SingleNodeCMSRediscoveryTest.java | 38 +++ 5 files changed, 338 insertions(+), 244 deletions(-) create mode 100644 test/distributed/org/apache/cassandra/distributed/test/log/AllClusterCMSRediscoveryTest.java create mode 100644 test/distributed/org/apache/cassandra/distributed/test/log/CMSRediscoveryTestBase.java delete mode 100644 test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java create mode 100644 test/distributed/org/apache/cassandra/distributed/test/log/MultiNodeCMSRediscoveryTest.java create mode 100644 test/distributed/org/apache/cassandra/distributed/test/log/SingleNodeCMSRediscoveryTest.java diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/AllClusterCMSRediscoveryTest.java b/test/distributed/org/apache/cassandra/distributed/test/log/AllClusterCMSRediscoveryTest.java new file mode 100644 index 000000000000..f508cedf9059 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/log/AllClusterCMSRediscoveryTest.java @@ -0,0 +1,38 @@ +/* + * 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.log; + +/** + * Tests a full-cluster restart with every node's broadcast address changing while down. + * The cluster is configured with 3 nodes, all of which are CMS members. + */ +public class AllClusterCMSRediscoveryTest extends CMSRediscoveryTestBase +{ + @Override + int clusterSize() + { + return 3; + } + + @Override + int cmsSize() + { + return 3; + } +} diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/CMSRediscoveryTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/log/CMSRediscoveryTestBase.java new file mode 100644 index 000000000000..364fdcb41f27 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/log/CMSRediscoveryTestBase.java @@ -0,0 +1,224 @@ +/* + * 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.log; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import com.google.common.base.Throwables; + +import org.awaitility.Awaitility; +import org.junit.Before; +import org.junit.Test; + +import org.apache.cassandra.config.CassandraRelevantProperties; +import org.apache.cassandra.cql3.QueryProcessor; +import org.apache.cassandra.cql3.UntypedResultSet; +import org.apache.cassandra.distributed.Cluster; +import org.apache.cassandra.distributed.api.IInvokableInstance; +import org.apache.cassandra.distributed.shared.ClusterUtils; +import org.apache.cassandra.distributed.test.TestBaseImpl; +import org.apache.cassandra.tcm.Epoch; +import org.apache.cassandra.tcm.membership.NodeId; +import org.apache.cassandra.utils.FBUtilities; +import org.apache.cassandra.utils.Pair; + +import static org.apache.cassandra.distributed.api.Feature.GOSSIP; +import static org.apache.cassandra.distributed.api.Feature.NETWORK; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Test the rediscovery and recovery of a cluster where every node is stopped and restarted with a new broadcast + * address. Because the routing info (i.e. broadcast addresses) are persisted in ClusterMetadata and changing them + * requires a majority of CMS members to commit a transformation to the metadata log, there is a chicken and egg issue + * when the CMS members cannot locate each other to form a quorum. + */ +public abstract class CMSRediscoveryTestBase extends TestBaseImpl +{ + + @Before + public void disableAccord() + { + CassandraRelevantProperties.DTEST_ACCORD_ENABLED.setBoolean(false); + } + + abstract int clusterSize(); + abstract int cmsSize(); + + @Test + public void testNewCMSDiscovery() throws IOException, ExecutionException, InterruptedException + { + int clusterSize = clusterSize(); + int cmsSize = cmsSize(); + try (Cluster cluster = builder().withNodes(clusterSize) + .withConfig(config -> config.with(NETWORK, GOSSIP) + .set("progress_barrier_backoff", "50ms")) + .createWithoutStarting()) + { + ExecutorService executor = Executors.newFixedThreadPool(clusterSize); + // Some fetchCMSLog operations might temporarily fail and be retried during address changes + String[] expectedErrors = new String[]{ + "Cannot achieve consistency level SERIAL.*", + "Queried for epoch Epoch\\{epoch=\\d+\\}, but could not catch up.*" + }; + cluster.setUncaughtExceptionsFilter((node, t) -> { + Throwable rootCause = Throwables.getRootCause(t); + if (rootCause.getMessage() == null) + return false; + String message = rootCause.getMessage(); + for (String expected : expectedErrors) + if (message.matches(expected)) + return true; + return false; + }); + cluster.startup(); + init(cluster); + IInvokableInstance n1 = cluster.get(1); + if (cmsSize > 1) + n1.nodetoolResult("cms", "reconfigure", "" + cmsSize).asserts().success(); + ClusterUtils.waitForCMSToQuiesce(cluster, n1); + + // Set up the expectations for what address changes are going to happen + Map> addressMapping = new HashMap<>(cluster.size()); + for (IInvokableInstance inst : cluster) + { + InetSocketAddress starting = inst.config().broadcastAddress(); + InetSocketAddress expected = new InetSocketAddress(bumpAddress(starting.getAddress()), starting.getPort()); + NodeId id = ClusterUtils.getNodeId(inst); + addressMapping.put(starting, Pair.create(id, expected)); + } + + // Check the CMS membership at the start of the test & predict what it should be at the end + Set startingCMS = new HashSet<>(cmsSize); + Set expectedCMS = new HashSet<>(cmsSize); + for (InetSocketAddress s : ClusterUtils.getCMSMemberAddresses(n1)) + { + startingCMS.add(s); + expectedCMS.add(addressMapping.get(s).right); + } + Set cmsNodes = ClusterUtils.getCMSMemberIds(n1); + + // Shut down all nodes, modify each one's broadcast address and reconfigure seeds as these + // will be used to rediscover peers. Seed config does not need to be uniform across the cluster + // but there must be enough intersection to enable the CMS members to rediscover each other + for (int i = 1; i <= cluster.size(); i++) + { + IInvokableInstance inst = cluster.get(i); + inst.shutdown().get(); + InetAddress newBroadcastAddress = addressMapping.get(inst.config().broadcastAddress()).right.getAddress(); + byte[] bytes = newBroadcastAddress.getAddress(); + ClusterUtils.updateAddress(inst, addrString(bytes)); + String seed1 = addrString(bytes[0], bytes[1], bytes[2], (byte) i); + String seed2 = addrString(bytes[0], bytes[1], bytes[2], (byte) ((i < cluster.size()) ? i + 1 : 1)); + ClusterUtils.updateSeed(inst, seed1, seed2); + } + + // Start everything up and wait for state to cluster state to quiesce + List> startups = new ArrayList<>(cluster.size()); + for (IInvokableInstance inst : cluster) + { + Future f = executor.submit(() -> { + inst.startup(); + return true; + }); + startups.add(f); + } + + FBUtilities.waitOnFutures(startups, 60, TimeUnit.SECONDS); + ClusterUtils.waitForCMSToQuiesce(cluster, n1); + + // wait until each node's STARTUP transformation has been enacted by all nodes + Awaitility.waitAtMost(30, TimeUnit.SECONDS).until(() -> allAddressChangesEnacted(cluster)); + Epoch afterAllAddressChanges = getEpochAfterAllAddressChanges(cluster); + ClusterUtils.waitForCMSToQuiesce(cluster, afterAllAddressChanges, true); + + // Assert that: + // * The membership of the CMS (i.e. which node ids) remains the same + // * The set of CMS addresses matches the prediction made at the start + // * Every node has successfully changed its address. The previous check + // is a logical consequence of this, but it doesn't hurt to verify both + for (IInvokableInstance inst : cluster) + { + assertEquals(cmsNodes, ClusterUtils.getCMSMemberIds(inst)); + Set finalCMS = ClusterUtils.getCMSMemberAddresses(inst); + assertEquals(startingCMS.size(), finalCMS.size()); + assertEquals(expectedCMS.size(), finalCMS.size()); + assertTrue(expectedCMS.containsAll(finalCMS)); + for (Pair peer : addressMapping.values()) + { + InetSocketAddress fromInst = ClusterUtils.getEndpoint(inst, peer.left); + assertEquals("Check failed on instance " + inst.config().num(), peer.right, fromInst); + } + } + + cluster.schemaChange("Create keyspace ks1 with replication = {'class': 'SimpleStrategy', 'replication_factor': 1}"); + cluster.schemaChange("Create keyspace ks2 with replication = {'class': 'SimpleStrategy', 'replication_factor': 1}"); + cluster.schemaChange("Create keyspace ks3 with replication = {'class': 'SimpleStrategy', 'replication_factor': 1}"); + } + } + + private boolean allAddressChangesEnacted(Cluster cluster) + { + return getEpochAfterAllAddressChanges(cluster).isAfter(Epoch.FIRST); + } + + private Epoch getEpochAfterAllAddressChanges(Cluster cluster) + { + int nodes = cluster.size(); + long epochAfterAllStartups = cluster.get(1).callOnInstance(() -> { + UntypedResultSet rs = QueryProcessor.executeInternal("SELECT epoch, kind from system_views.cluster_metadata_log where kind = 'STARTUP'"); + if (rs == null || rs.isEmpty() || rs.size() < nodes) + return -1; + + long epoch = rs.stream().mapToLong(r -> r.getLong("epoch")).max().orElse(-1); + return epoch; + }).longValue(); + return Epoch.create(epochAfterAllStartups); + } + + private InetAddress bumpAddress(InetAddress address) throws UnknownHostException + { + // ipv4 addresses for this test + assert address.getAddress().length == 4; + byte[] bytes = address.getAddress(); + bytes[2]++; + return InetAddress.getByAddress(bytes); + } + + private String addrString(byte...b) throws UnknownHostException + { + assert b.length == 4; + return InetAddress.getByAddress(new byte[] { b[0], b[1], b[2], b[3] }).getHostAddress(); + } + +} diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java b/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java deleted file mode 100644 index d2bdbd31b354..000000000000 --- a/test/distributed/org/apache/cassandra/distributed/test/log/DiscoverNewCMSTest.java +++ /dev/null @@ -1,244 +0,0 @@ -/* - * 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.log; - -import java.io.IOException; -import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.net.UnknownHostException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; - -import com.google.common.base.Throwables; - -import org.awaitility.Awaitility; -import org.junit.Before; -import org.junit.Test; - -import org.apache.cassandra.config.CassandraRelevantProperties; -import org.apache.cassandra.cql3.QueryProcessor; -import org.apache.cassandra.cql3.UntypedResultSet; -import org.apache.cassandra.distributed.Cluster; -import org.apache.cassandra.distributed.api.IInvokableInstance; -import org.apache.cassandra.distributed.shared.ClusterUtils; -import org.apache.cassandra.distributed.test.TestBaseImpl; -import org.apache.cassandra.tcm.Epoch; -import org.apache.cassandra.tcm.membership.NodeId; -import org.apache.cassandra.utils.FBUtilities; -import org.apache.cassandra.utils.Pair; - -import static org.apache.cassandra.distributed.api.Feature.GOSSIP; -import static org.apache.cassandra.distributed.api.Feature.NETWORK; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -public class DiscoverNewCMSTest extends TestBaseImpl -{ - - @Before - public void disableAccord() - { - CassandraRelevantProperties.DTEST_ACCORD_ENABLED.setBoolean(false); - } - - @Test - public void singleNodeCMSAddressChangeTest() throws IOException, ExecutionException, InterruptedException - { - try (Cluster cluster = builder().withNodes(3) - .withConfig(config -> config.with(NETWORK, GOSSIP) - .set("progress_barrier_backoff", "50ms")) - .createWithoutStarting()) - { - test(cluster, 1); - } - } - - @Test - public void multiNodeCMSOnlyClusterAddressChangeTest() throws IOException, ExecutionException, InterruptedException - { - - try (Cluster cluster = builder().withNodes(3) - .withConfig(config -> config.with(NETWORK, GOSSIP) - .set("progress_barrier_backoff", "50ms")) - .createWithoutStarting()) - { - test(cluster, 3); - } - } - - @Test - public void multiNodeCMSAllAddressesChangeTest() throws IOException, ExecutionException, InterruptedException - { - try (Cluster cluster = builder().withNodes(6) - .withConfig(config -> config.with(NETWORK, GOSSIP) - .set("progress_barrier_backoff", "50ms")) - .createWithoutStarting()) - { - test(cluster, 3); - } - } - - private void test(Cluster cluster, int cmsSize) throws IOException, ExecutionException, InterruptedException - { - ExecutorService executor = Executors.newFixedThreadPool(cluster.size()); - // Some fetchCMSLog operations might temporarily fail and be retried during address changes - String[] expectedErrors = new String[] { - "Cannot achieve consistency level SERIAL.*", - "Queried for epoch Epoch\\{epoch=\\d+\\}, but could not catch up.*" - }; - cluster.setUncaughtExceptionsFilter((node, t) -> { - Throwable rootCause = Throwables.getRootCause(t); - if (rootCause.getMessage() == null) - return false; - String message = rootCause.getMessage(); - for (String expected : expectedErrors) - if (message.matches(expected)) - return true; - return false; - }); - cluster.startup(); - init(cluster); - IInvokableInstance n1 = cluster.get(1); - if (cmsSize > 1) - n1.nodetoolResult("cms", "reconfigure", "" + cmsSize).asserts().success(); - ClusterUtils.waitForCMSToQuiesce(cluster, n1); - - // Set up the expectations for what address changes are going to happen - Map> addressMapping = new HashMap<>(cluster.size()); - for (IInvokableInstance inst : cluster) - { - InetSocketAddress starting = inst.config().broadcastAddress(); - InetSocketAddress expected = new InetSocketAddress(bumpAddress(starting.getAddress()), starting.getPort()); - NodeId id = ClusterUtils.getNodeId(inst); - addressMapping.put(starting, Pair.create(id, expected)); - } - - // Check the CMS membership at the start of the test & predict what it should be at the end - Set startingCMS = new HashSet<>(cmsSize); - Set expectedCMS = new HashSet<>(cmsSize); - for (InetSocketAddress s : ClusterUtils.getCMSMemberAddresses(n1)) - { - startingCMS.add(s); - expectedCMS.add(addressMapping.get(s).right); - } - Set cmsNodes = ClusterUtils.getCMSMemberIds(n1); - - // Shut down all nodes, modify each one's broadcast address and reconfigure seeds as these - // will be used to rediscover peers. Seed config does not need to be uniform across the cluster - // but there must be enough intersection to enable the CMS members to rediscover each other - for (int i = 1; i <= cluster.size(); i++) - { - IInvokableInstance inst = cluster.get(i); - inst.shutdown().get(); - InetAddress newBroadcastAddress = addressMapping.get(inst.config().broadcastAddress()).right.getAddress(); - byte[] bytes = newBroadcastAddress.getAddress(); - ClusterUtils.updateAddress(inst, addrString(bytes)); - String seed1 = addrString(bytes[0], bytes[1], bytes[2], (byte) i); - String seed2 = addrString(bytes[0], bytes[1], bytes[2], (byte) ((i < cluster.size()) ? i + 1 : 1)); - ClusterUtils.updateSeed(inst, seed1, seed2); - } - - // Start everything up and wait for state to cluster state to quiesce - List> startups = new ArrayList<>(cluster.size()); - for (IInvokableInstance inst : cluster) - { - Future f = executor.submit(() -> { - inst.startup(); - return true; - }); - startups.add(f); - } - - FBUtilities.waitOnFutures(startups, 60, TimeUnit.SECONDS); - ClusterUtils.waitForCMSToQuiesce(cluster, n1); - - // wait until each node's STARTUP transformation has been enacted by all nodes - Awaitility.waitAtMost(30, TimeUnit.SECONDS).until(() -> allAddressChangesEnacted(cluster)); - Epoch afterAllAddressChanges = getEpochAfterAllAddressChanges(cluster); - ClusterUtils.waitForCMSToQuiesce(cluster, afterAllAddressChanges, true); - - // Assert that: - // * The membership of the CMS (i.e. which node ids) remains the same - // * The set of CMS addresses matches the prediction made at the start - // * Every node has successfully changed its address. The previous check - // is a logical consequence of this, but it doesn't hurt to verify both - for (IInvokableInstance inst : cluster) - { - assertEquals(cmsNodes, ClusterUtils.getCMSMemberIds(inst)); - Set finalCMS = ClusterUtils.getCMSMemberAddresses(inst); - assertEquals(startingCMS.size(), finalCMS.size()); - assertEquals(expectedCMS.size(), finalCMS.size()); - assertTrue(expectedCMS.containsAll(finalCMS)); - for (Pair peer : addressMapping.values()) - { - InetSocketAddress fromInst = ClusterUtils.getEndpoint(inst, peer.left); - assertEquals("Check failed on instance " + inst.config().num(), peer.right, fromInst); - } - } - - cluster.schemaChange("Create keyspace ks1 with replication = {'class': 'SimpleStrategy', 'replication_factor': 1}"); - cluster.schemaChange("Create keyspace ks2 with replication = {'class': 'SimpleStrategy', 'replication_factor': 1}"); - cluster.schemaChange("Create keyspace ks3 with replication = {'class': 'SimpleStrategy', 'replication_factor': 1}"); - - } - - private boolean allAddressChangesEnacted(Cluster cluster) - { - return getEpochAfterAllAddressChanges(cluster).isAfter(Epoch.FIRST); - } - - private Epoch getEpochAfterAllAddressChanges(Cluster cluster) - { - int nodes = cluster.size(); - long epochAfterAllStartups = cluster.get(1).callOnInstance(() -> { - UntypedResultSet rs = QueryProcessor.executeInternal("SELECT epoch, kind from system_views.cluster_metadata_log where kind = 'STARTUP'"); - if (rs == null || rs.isEmpty() || rs.size() < nodes) - return -1; - - long epoch = rs.stream().mapToLong(r -> r.getLong("epoch")).max().orElse(-1); - return epoch; - }).longValue(); - return Epoch.create(epochAfterAllStartups); - } - - private InetAddress bumpAddress(InetAddress address) throws UnknownHostException - { - // ipv4 addresses for this test - assert address.getAddress().length == 4; - byte[] bytes = address.getAddress(); - bytes[2]++; - return InetAddress.getByAddress(bytes); - } - - private String addrString(byte...b) throws UnknownHostException - { - assert b.length == 4; - return InetAddress.getByAddress(new byte[] { b[0], b[1], b[2], b[3] }).getHostAddress(); - } - -} diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/MultiNodeCMSRediscoveryTest.java b/test/distributed/org/apache/cassandra/distributed/test/log/MultiNodeCMSRediscoveryTest.java new file mode 100644 index 000000000000..70e34b340204 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/log/MultiNodeCMSRediscoveryTest.java @@ -0,0 +1,38 @@ +/* + * 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.log; + +/** + * Tests a full-cluster restart with every node's broadcast address changing while down. + * The cluster is configured with 6 nodes, 3 of which are CMS members. + */ +public class MultiNodeCMSRediscoveryTest extends CMSRediscoveryTestBase +{ + @Override + int clusterSize() + { + return 6; + } + + @Override + int cmsSize() + { + return 3; + } +} diff --git a/test/distributed/org/apache/cassandra/distributed/test/log/SingleNodeCMSRediscoveryTest.java b/test/distributed/org/apache/cassandra/distributed/test/log/SingleNodeCMSRediscoveryTest.java new file mode 100644 index 000000000000..9de7ffaae916 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/log/SingleNodeCMSRediscoveryTest.java @@ -0,0 +1,38 @@ +/* + * 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.log; + +/** + * Tests a full-cluster restart with every node's broadcast address changing while down. + * The cluster is configured with 3 nodes, only 1 of which is a CMS member. + */ +public class SingleNodeCMSRediscoveryTest extends CMSRediscoveryTestBase +{ + @Override + int clusterSize() + { + return 3; + } + + @Override + int cmsSize() + { + return 1; + } +} From 665839ae9672cdad203b44fa032df1300542f108 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Wed, 29 Jul 2026 14:21:07 +0100 Subject: [PATCH 27/27] [CASSANDRA-20476] Ensure proper exit from NewGossiper shadow round after upgrade --- .../org/apache/cassandra/gms/NewGossiper.java | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/java/org/apache/cassandra/gms/NewGossiper.java b/src/java/org/apache/cassandra/gms/NewGossiper.java index 034afb9bfff7..fbb061795f5c 100644 --- a/src/java/org/apache/cassandra/gms/NewGossiper.java +++ b/src/java/org/apache/cassandra/gms/NewGossiper.java @@ -20,7 +20,6 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -60,8 +59,14 @@ public class NewGossiper public Map doShadowRound() { Set peers = new HashSet<>(SystemKeyspace.loadHostIds().keySet()); - peers.addAll(DatabaseDescriptor.getSeeds()); - if (peers.equals(Collections.singleton(getBroadcastAddressAndPort()))) + for (InetAddressAndPort seed : DatabaseDescriptor.getSeeds()) + { + if (!seed.equals(getBroadcastAddressAndPort())) + peers.add(seed); + } + + // implies a single node cluster with only that one node configured as a seed + if (peers.isEmpty()) return GossipHelper.storedEpstate(); ShadowRoundHandler shadowRoundHandler = new ShadowRoundHandler(peers); @@ -84,6 +89,9 @@ public Map doShadowRound() } } logger.warn("Not able to construct initial cluster metadata from gossip, using system tables instead"); + // Mark done here so that future gossip messages don't get routed to the shadow round handler (see + // GossipDigestSynVerbHandler & GossipDigestAckVerbHandler) + handler.markDone(); return GossipHelper.storedEpstate(); } @@ -119,10 +127,16 @@ public ShadowRoundHandler(Set peers, MessageDelivery message this.peers = ConcurrentHashMap.newKeySet(); this.peers.addAll(peers); responses = new ConcurrentHashMap<>(); - requiredResponses = this.peers.size() < 3 ? 1 : Math.max(this.peers.size() / 5, 2); // require response from 20% of the cluster + requiredResponses = this.peers.size() <= 3 ? 1 : Math.max(this.peers.size() / 5, 2); // require response from 20% of the cluster this.messageDelivery = messageDelivery; } + public void markDone() + { + logger.info("Marking NewGossiper shadow round done"); + isDone = true; + } + public boolean isDone() { return isDone;