From 91f29a1f866c15a522ece99d92ad44cb9d502c7d Mon Sep 17 00:00:00 2001 From: David Capwell Date: Thu, 17 Apr 2025 14:57:01 -0700 Subject: [PATCH] Accord: Hopefully last rebase cleanup patch by David Capwell; reviewed by Ariel Weisberg for CASSANDRA-20568 --- .../cassandra/exceptions/RequestFailure.java | 6 ++-- .../exceptions/RequestFailureReason.java | 25 +++++++++---- .../cassandra/service/StorageService.java | 3 +- .../accord/MigrationToAccordReadRaceTest.java | 8 +---- .../AccordInteropMultiNodeTableWalkBase.java | 8 ----- .../fuzz/sai/AccordFullMultiNodeSAITest.java | 3 ++ .../sai/AccordInteropMultiNodeSAITest.java | 3 ++ .../topology/AccordTopologyMixupTest.java | 2 +- .../cassandra/dht/BootStrapperTest.java | 36 ++++++++++--------- .../exceptions/RequestFailureReasonTest.java | 4 +-- .../apache/cassandra/utils/ASTGenerators.java | 1 + 11 files changed, 53 insertions(+), 46 deletions(-) diff --git a/src/java/org/apache/cassandra/exceptions/RequestFailure.java b/src/java/org/apache/cassandra/exceptions/RequestFailure.java index 9f6d0575ce..b9bba7fc70 100644 --- a/src/java/org/apache/cassandra/exceptions/RequestFailure.java +++ b/src/java/org/apache/cassandra/exceptions/RequestFailure.java @@ -43,7 +43,6 @@ import static org.apache.cassandra.exceptions.ExceptionSerializer.nullableRemote public class RequestFailure { public static final RequestFailure UNKNOWN = new RequestFailure(RequestFailureReason.UNKNOWN); - public static final RequestFailure ACCORD_DISABLED = new RequestFailure(RequestFailureReason.ACCORD_DISABLED); public static final RequestFailure READ_TOO_MANY_TOMBSTONES = new RequestFailure(RequestFailureReason.READ_TOO_MANY_TOMBSTONES); public static final RequestFailure TIMEOUT = new RequestFailure(RequestFailureReason.TIMEOUT); public static final RequestFailure INCOMPATIBLE_SCHEMA = new RequestFailure(RequestFailureReason.INCOMPATIBLE_SCHEMA); @@ -55,7 +54,7 @@ public class RequestFailure public static final RequestFailure COORDINATOR_BEHIND = new RequestFailure(RequestFailureReason.COORDINATOR_BEHIND); public static final RequestFailure READ_TOO_MANY_INDEXES = new RequestFailure(RequestFailureReason.READ_TOO_MANY_INDEXES); public static final RequestFailure RETRY_ON_DIFFERENT_TRANSACTION_SYSTEM = new RequestFailure(RequestFailureReason.RETRY_ON_DIFFERENT_TRANSACTION_SYSTEM); - public static final RequestFailure BOOTING = new RequestFailure(RequestFailureReason.BOOTING); + public static final RequestFailure INDEX_BUILD_IN_PROGRESS = new RequestFailure(RequestFailureReason.INDEX_BUILD_IN_PROGRESS); static { @@ -135,7 +134,6 @@ public class RequestFailure { default: throw new IllegalStateException("Unhandled request failure reason " + reason); case UNKNOWN: return UNKNOWN; - case ACCORD_DISABLED: return ACCORD_DISABLED; case READ_TOO_MANY_TOMBSTONES: return READ_TOO_MANY_TOMBSTONES; case TIMEOUT: return TIMEOUT; case INCOMPATIBLE_SCHEMA: return INCOMPATIBLE_SCHEMA; @@ -146,8 +144,8 @@ public class RequestFailure case INDEX_NOT_AVAILABLE: return INDEX_NOT_AVAILABLE; case COORDINATOR_BEHIND: return COORDINATOR_BEHIND; case READ_TOO_MANY_INDEXES: return READ_TOO_MANY_INDEXES; + case INDEX_BUILD_IN_PROGRESS: return INDEX_BUILD_IN_PROGRESS; case RETRY_ON_DIFFERENT_TRANSACTION_SYSTEM: return RETRY_ON_DIFFERENT_TRANSACTION_SYSTEM; - case BOOTING: return BOOTING; } } diff --git a/src/java/org/apache/cassandra/exceptions/RequestFailureReason.java b/src/java/org/apache/cassandra/exceptions/RequestFailureReason.java index 38b921eb6a..bafab71752 100644 --- a/src/java/org/apache/cassandra/exceptions/RequestFailureReason.java +++ b/src/java/org/apache/cassandra/exceptions/RequestFailureReason.java @@ -18,9 +18,12 @@ package org.apache.cassandra.exceptions; import java.io.IOException; +import java.util.EnumSet; import java.util.HashMap; import java.util.Map; +import com.google.common.collect.Sets; + import org.apache.cassandra.db.filter.TombstoneOverwhelmingException; import org.apache.cassandra.index.IndexBuildInProgressException; import org.apache.cassandra.index.IndexNotAvailableException; @@ -47,11 +50,9 @@ public enum RequestFailureReason NOT_CMS (8), INVALID_ROUTING (9), COORDINATOR_BEHIND (10), + RETRY_ON_DIFFERENT_TRANSACTION_SYSTEM (11), // The following codes have been ported from an external fork, where they were offset explicitly to avoid conflicts. INDEX_BUILD_IN_PROGRESS (503), - RETRY_ON_DIFFERENT_TRANSACTION_SYSTEM (504), - BOOTING (505), - ACCORD_DISABLED (506) ; static @@ -71,10 +72,11 @@ public enum RequestFailureReason private static final Map codeToReasonMap = new HashMap<>(); private static final Map, RequestFailureReason> exceptionToReasonMap = new HashMap<>(); - private static final int REASONS_WITHOUT_EXCEPTIONS = 3; // UNKNOWN, NODE_DOWN, and READ_TOO_MANY_INDEXES static { + EnumSet withoutExceptions = EnumSet.of(UNKNOWN, NODE_DOWN, READ_TOO_MANY_INDEXES); + Sets.SetView withExceptions = Sets.difference(EnumSet.allOf(RequestFailureReason.class), withoutExceptions); RequestFailureReason[] reasons = values(); for (RequestFailureReason reason : reasons) @@ -92,9 +94,20 @@ public enum RequestFailureReason exceptionToReasonMap.put(InvalidRoutingException.class, INVALID_ROUTING); exceptionToReasonMap.put(CoordinatorBehindException.class, COORDINATOR_BEHIND); exceptionToReasonMap.put(IndexBuildInProgressException.class, INDEX_BUILD_IN_PROGRESS); + exceptionToReasonMap.put(RetryOnDifferentSystemException.class, RETRY_ON_DIFFERENT_TRANSACTION_SYSTEM); - if (exceptionToReasonMap.size() != reasons.length - REASONS_WITHOUT_EXCEPTIONS) - throw new RuntimeException("A new RequestFailureReasons was probably added and you may need to update the exceptionToReasonMap"); + if (exceptionToReasonMap.size() != reasons.length - withoutExceptions.size()) + { + EnumSet actual = EnumSet.copyOf(exceptionToReasonMap.values()); + Sets.SetView missing = Sets.difference(withExceptions, actual); + Sets.SetView added = Sets.difference(actual, withExceptions); + StringBuilder sb = new StringBuilder(); + if (!missing.isEmpty()) + sb.append("Expected the following RequestFailureReason, but were missing: ").append(missing).append('\n'); + if (!added.isEmpty()) + sb.append("Unexpected RequestFailureReason found: ").append(added); + throw new AssertionError(sb.toString()); + } } public static RequestFailureReason fromCode(int code) diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 90e2f598d2..c97b2ec695 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -4015,12 +4015,13 @@ public class StorageService extends NotificationBroadcasterSupport implements IE // Never ever do this at home. Used by tests. @VisibleForTesting - public void setPartitionerUnsafe(IPartitioner newPartitioner) + public IPartitioner setPartitionerUnsafe(IPartitioner newPartitioner) { checkNotNull(newPartitioner, "newPartitioner is null"); checkState(originalPartitioner == null, "Already changed the partitioner without resetting"); originalPartitioner = DatabaseDescriptor.setPartitionerUnsafe(newPartitioner); valueFactory = new VersionedValue.VersionedValueFactory(newPartitioner); + return originalPartitioner; } @VisibleForTesting diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/MigrationToAccordReadRaceTest.java b/test/distributed/org/apache/cassandra/distributed/test/accord/MigrationToAccordReadRaceTest.java index 3c47fa5871..4faa91ebce 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/MigrationToAccordReadRaceTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/MigrationToAccordReadRaceTest.java @@ -20,6 +20,7 @@ package org.apache.cassandra.distributed.test.accord; import org.junit.Ignore; +@Ignore("Flakey") public class MigrationToAccordReadRaceTest extends AccordMigrationReadRaceTestBase { @Override @@ -27,11 +28,4 @@ public class MigrationToAccordReadRaceTest extends AccordMigrationReadRaceTestBa { return false; } - - @Ignore - @Override - public void testBounds() throws Throwable - { - super.testBounds(); - } } diff --git a/test/distributed/org/apache/cassandra/distributed/test/cql3/AccordInteropMultiNodeTableWalkBase.java b/test/distributed/org/apache/cassandra/distributed/test/cql3/AccordInteropMultiNodeTableWalkBase.java index 332b2e2b21..6e4c1ad1bd 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/cql3/AccordInteropMultiNodeTableWalkBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/cql3/AccordInteropMultiNodeTableWalkBase.java @@ -23,7 +23,6 @@ import accord.utils.RandomSource; import org.apache.cassandra.cql3.KnownIssue; import org.apache.cassandra.distributed.Cluster; import org.apache.cassandra.distributed.api.ConsistencyLevel; -import org.apache.cassandra.distributed.api.IInstanceConfig; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.service.consensus.TransactionalMode; import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; @@ -69,13 +68,6 @@ Suppressed: java.lang.AssertionError: Unknown keyspace ks12 } } - @Override - protected void clusterConfig(IInstanceConfig c) - { - super.clusterConfig(c); - c.set("transaction_timeout", "180s"); - } - @Override protected TableMetadata defineTable(RandomSource rs, String ks) { diff --git a/test/distributed/org/apache/cassandra/fuzz/sai/AccordFullMultiNodeSAITest.java b/test/distributed/org/apache/cassandra/fuzz/sai/AccordFullMultiNodeSAITest.java index f2a94901a6..23b52833af 100644 --- a/test/distributed/org/apache/cassandra/fuzz/sai/AccordFullMultiNodeSAITest.java +++ b/test/distributed/org/apache/cassandra/fuzz/sai/AccordFullMultiNodeSAITest.java @@ -18,11 +18,14 @@ package org.apache.cassandra.fuzz.sai; +import org.junit.Ignore; + import org.apache.cassandra.harry.SchemaSpec; import org.apache.cassandra.harry.gen.Generator; import org.apache.cassandra.harry.gen.SchemaGenerators; import org.apache.cassandra.service.consensus.TransactionalMode; +@Ignore("CASSANDRA-20567: Repair is failing due to missing SAI index files when using zero copy streaming") public class AccordFullMultiNodeSAITest extends MultiNodeSAITestBase { public AccordFullMultiNodeSAITest() diff --git a/test/distributed/org/apache/cassandra/fuzz/sai/AccordInteropMultiNodeSAITest.java b/test/distributed/org/apache/cassandra/fuzz/sai/AccordInteropMultiNodeSAITest.java index 6d507ecd5f..ba937c0f91 100644 --- a/test/distributed/org/apache/cassandra/fuzz/sai/AccordInteropMultiNodeSAITest.java +++ b/test/distributed/org/apache/cassandra/fuzz/sai/AccordInteropMultiNodeSAITest.java @@ -18,11 +18,14 @@ package org.apache.cassandra.fuzz.sai; +import org.junit.Ignore; + import org.apache.cassandra.harry.SchemaSpec; import org.apache.cassandra.harry.gen.Generator; import org.apache.cassandra.harry.gen.SchemaGenerators; import org.apache.cassandra.service.consensus.TransactionalMode; +@Ignore("CASSANDRA-20567: Repair is failing due to missing SAI index files when using zero copy streaming") public class AccordInteropMultiNodeSAITest extends MultiNodeSAITestBase { public AccordInteropMultiNodeSAITest() diff --git a/test/distributed/org/apache/cassandra/fuzz/topology/AccordTopologyMixupTest.java b/test/distributed/org/apache/cassandra/fuzz/topology/AccordTopologyMixupTest.java index 5fbb93efca..8d6bc02773 100644 --- a/test/distributed/org/apache/cassandra/fuzz/topology/AccordTopologyMixupTest.java +++ b/test/distributed/org/apache/cassandra/fuzz/topology/AccordTopologyMixupTest.java @@ -148,7 +148,7 @@ public class AccordTopologyMixupTest extends TopologyMixupTestBase cqlOperations(Spec spec) { Gen select = (Gen) (Gen) fromQT(new ASTGenerators.SelectGenBuilder(spec.metadata).withLimit1().build()); - Gen mutation = (Gen) (Gen) fromQT(new ASTGenerators.MutationGenBuilder(spec.metadata).withoutTimestamp().withoutTtl().build()); + Gen mutation = (Gen) (Gen) fromQT(new ASTGenerators.MutationGenBuilder(spec.metadata).withoutTimestamp().withoutTtl().withAllowUpdateMultipleClusteringKeys(false).build()); Gen txn = (Gen) (Gen) fromQT(new ASTGenerators.TxnGenBuilder(spec.metadata).build()); Map, Integer> operations = new LinkedHashMap<>(); operations.put(select, 1); diff --git a/test/unit/org/apache/cassandra/dht/BootStrapperTest.java b/test/unit/org/apache/cassandra/dht/BootStrapperTest.java index 9739d3ed59..c139acbe3c 100644 --- a/test/unit/org/apache/cassandra/dht/BootStrapperTest.java +++ b/test/unit/org/apache/cassandra/dht/BootStrapperTest.java @@ -33,10 +33,8 @@ import com.google.common.collect.Multimap; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; +import org.junit.runner.RunWith; -import org.apache.cassandra.CassandraTestBase; -import org.apache.cassandra.CassandraTestBase.PrepareServerNoRegister; -import org.apache.cassandra.CassandraTestBase.UseMurmur3Partitioner; import org.apache.cassandra.SchemaLoader; import org.apache.cassandra.ServerTestUtils; import org.apache.cassandra.config.CassandraRelevantProperties; @@ -50,6 +48,7 @@ import org.apache.cassandra.locator.InetAddressAndPort; import org.apache.cassandra.locator.Replica; import org.apache.cassandra.schema.Schema; import org.apache.cassandra.schema.SchemaConstants; +import org.apache.cassandra.service.StorageService; import org.apache.cassandra.streaming.StreamOperation; import org.apache.cassandra.tcm.ClusterMetadata; import org.apache.cassandra.tcm.membership.NodeId; @@ -58,15 +57,16 @@ import org.apache.cassandra.tcm.sequences.BootstrapAndJoin; import org.apache.cassandra.utils.Pair; import org.jboss.byteman.contrib.bmunit.BMRule; import org.jboss.byteman.contrib.bmunit.BMRules; +import org.jboss.byteman.contrib.bmunit.BMUnitRunner; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -@UseMurmur3Partitioner -@PrepareServerNoRegister -public class BootStrapperTest extends CassandraTestBase +@RunWith(BMUnitRunner.class) +public class BootStrapperTest { + static IPartitioner oldPartitioner; static Predicate originalAlivePredicate = RangeStreamer.ALIVE_PREDICATE; public static AtomicBoolean nonOptimizationHit = new AtomicBoolean(false); public static AtomicBoolean optimizationHit = new AtomicBoolean(false); @@ -88,6 +88,9 @@ public class BootStrapperTest extends CassandraTestBase @BeforeClass public static void setup() throws ConfigurationException { + DatabaseDescriptor.daemonInitialization(); + oldPartitioner = StorageService.instance.setPartitionerUnsafe(Murmur3Partitioner.instance); + ServerTestUtils.prepareServerNoRegister(); SchemaLoader.startGossiper(); SchemaLoader.schemaDefinition("BootStrapperTest"); RangeStreamer.ALIVE_PREDICATE = Predicates.alwaysTrue(); @@ -97,6 +100,7 @@ public class BootStrapperTest extends CassandraTestBase @AfterClass public static void tearDown() { + DatabaseDescriptor.setPartitionerUnsafe(oldPartitioner); RangeStreamer.ALIVE_PREDICATE = originalAlivePredicate; } @@ -204,16 +208,16 @@ public class BootStrapperTest extends CassandraTestBase } return new RangeStreamer(metadata, - StreamOperation.BOOTSTRAP, - true, - DatabaseDescriptor.getNodeProximity(), - new StreamStateStore(), - mockFailureDetector, - false, - 1, - movements.left, - movements.right, - true); + StreamOperation.BOOTSTRAP, + true, + DatabaseDescriptor.getNodeProximity(), + new StreamStateStore(), + mockFailureDetector, + false, + 1, + movements.left, + movements.right, + true); } private boolean includesWraparound(Collection> toFetch) diff --git a/test/unit/org/apache/cassandra/exceptions/RequestFailureReasonTest.java b/test/unit/org/apache/cassandra/exceptions/RequestFailureReasonTest.java index 9162a87e85..4be82d491c 100644 --- a/test/unit/org/apache/cassandra/exceptions/RequestFailureReasonTest.java +++ b/test/unit/org/apache/cassandra/exceptions/RequestFailureReasonTest.java @@ -40,10 +40,8 @@ public class RequestFailureReasonTest { 8, "NOT_CMS" }, { 9, "INVALID_ROUTING" }, { 10, "COORDINATOR_BEHIND" }, + { 11, "RETRY_ON_DIFFERENT_TRANSACTION_SYSTEM" }, { 503, "INDEX_BUILD_IN_PROGRESS" }, - { 504, "RETRY_ON_DIFFERENT_TRANSACTION_SYSTEM" }, - { 505, "BOOTING" }, - { 506, "ACCORD_DISABLED" } }; @Test diff --git a/test/unit/org/apache/cassandra/utils/ASTGenerators.java b/test/unit/org/apache/cassandra/utils/ASTGenerators.java index 1279f60cb7..3e1a228db1 100644 --- a/test/unit/org/apache/cassandra/utils/ASTGenerators.java +++ b/test/unit/org/apache/cassandra/utils/ASTGenerators.java @@ -1004,6 +1004,7 @@ public class ASTGenerators .withoutCas() .withoutTimestamp() .withoutTtl() + .withAllowUpdateMultipleClusteringKeys(false) .withReferences(new ArrayList<>(builder.allowedReferences())); if (!allowReferences) mutationBuilder.withReferences(Collections.emptyList());