From d422eb1f353d27264665bfe3357dac1160814ea1 Mon Sep 17 00:00:00 2001 From: Jacek Lewandowski Date: Fri, 8 Dec 2023 10:41:50 +0100 Subject: [PATCH] Fix storage_compatibility_mode for streaming - Rename and refactor a bit property for overriding storage compatibility mode - Fix streaming for tools - consider bulkloader as a client instead of tool Patch by Jacek Lewandowski; reviewed by Berenguer Blasi, Branimir Lambov for CASSANDRA-19126 --- CHANGES.txt | 1 + build.xml | 4 +-- .../config/CassandraRelevantProperties.java | 17 +++++------ .../org/apache/cassandra/config/Config.java | 2 +- .../cassandra/config/DatabaseDescriptor.java | 22 +++++++++++---- .../cassandra/net/MessagingService.java | 28 ++++++++++++++----- .../net/OutboundConnectionSettings.java | 2 +- .../repair/messages/PrepareMessage.java | 4 ++- .../apache/cassandra/tools/BulkLoader.java | 2 +- .../apache/cassandra/repair/FuzzTestBase.java | 2 +- 10 files changed, 56 insertions(+), 28 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 1d71cb52c3..485b945791 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.0-beta2 + * Fix storage_compatibility_mode for streaming (CASSANDRA-19126) * Add support of vector type to cqlsh COPY command (CASSANDRA-19118) * Make CQLSSTableWriter to support building of SAI indexes (CASSANDRA-18714) * Append additional JVM options when using JDK17+ (CASSANDRA-19001) diff --git a/build.xml b/build.xml index a701786f84..b7c8d1336a 100644 --- a/build.xml +++ b/build.xml @@ -1310,7 +1310,7 @@ - + @@ -1331,7 +1331,7 @@ - + diff --git a/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java b/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java index 16a24fd2f0..b3d0b52abe 100644 --- a/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java +++ b/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java @@ -316,14 +316,6 @@ public enum CassandraRelevantProperties /** Java Virtual Machine implementation name */ JAVA_VM_NAME("java.vm.name"), JOIN_RING("cassandra.join_ring", "true"), - /** - * {@link StorageCompatibilityMode} mode sets how the node will behave, sstable or messaging versions to use etc according to a yaml setting. - * But many tests don't load the config hence we need to force it otherwise they would run always under the default. Config is null for junits - * that don't load the config. Get from env var that CI/build.xml sets. - * - * This is a dev/CI only property. Do not use otherwise. - */ - JUNIT_STORAGE_COMPATIBILITY_MODE("cassandra.junit_storage_compatibility_mode", StorageCompatibilityMode.CASSANDRA_4.toString()), /** startup checks properties */ LIBJEMALLOC("cassandra.libjemalloc"), /** Line separator ("\n" on UNIX). */ @@ -562,6 +554,15 @@ public enum CassandraRelevantProperties TEST_SIMULATOR_PRINT_ASM_TYPES("cassandra.test.simulator.print_asm_types", ""), TEST_SKIP_CRYPTO_PROVIDER_INSTALLATION("cassandra.test.security.skip.provider.installation", "false"), TEST_SSTABLE_FORMAT_DEVELOPMENT("cassandra.test.sstableformatdevelopment"), + /** + * {@link StorageCompatibilityMode} mode sets how the node will behave, sstable or messaging versions to use etc. + * according to a yaml setting. But many tests don't load the config hence we need to force it otherwise they would + * run always under the default. Config is null for junits that don't load the config. Get from env var that + * CI/build.xml sets. + * + * This is a dev/CI only property. Do not use otherwise. + */ + TEST_STORAGE_COMPATIBILITY_MODE("cassandra.test.storage_compatibility_mode", StorageCompatibilityMode.CASSANDRA_4.toString()), TEST_STRICT_LCS_CHECKS("cassandra.test.strict_lcs_checks"), /** Turns some warnings into exceptions for testing. */ TEST_STRICT_RUNTIME_CHECKS("cassandra.strict.runtime.checks"), diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index d5b91ef530..9c3c941c5e 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -1269,5 +1269,5 @@ public class Config public double severity_during_decommission = 0; - public StorageCompatibilityMode storage_compatibility_mode = StorageCompatibilityMode.CASSANDRA_4; + public StorageCompatibilityMode storage_compatibility_mode; } diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 2d973d9b80..c8022bdaf5 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -228,6 +228,7 @@ public class DatabaseDescriptor private static ImmutableMap> sstableFormats; private static volatile SSTableFormat selectedSSTableFormat; + private static StorageCompatibilityMode storageCompatibilityMode = CassandraRelevantProperties.TEST_STORAGE_COMPATIBILITY_MODE.getEnum(true, StorageCompatibilityMode.class); private static Function commitLogSegmentMgrProvider = c -> DatabaseDescriptor.isCDCEnabled() ? new CommitLogSegmentManagerCDC(c, DatabaseDescriptor.getCommitLogLocation()) @@ -291,6 +292,8 @@ public class DatabaseDescriptor setConfig(loadConfig()); + applyCompatibilityMode(); + applySSTableFormats(); applySimpleConfig(); @@ -346,6 +349,7 @@ public class DatabaseDescriptor setDefaultFailureDetector(); Config.setClientMode(true); conf = configSupplier.get(); + applyCompatibilityMode(); diskOptimizationStrategy = new SpinningDiskOptimizationStrategy(); applySSTableFormats(); } @@ -434,7 +438,8 @@ public class DatabaseDescriptor private static void applyAll() throws ConfigurationException { - //InetAddressAndPort cares that applySimpleConfig runs first + applyCompatibilityMode(); + applySSTableFormats(); applyCryptoProvider(); @@ -1521,6 +1526,15 @@ public class DatabaseDescriptor return selectedFormat; } + private static void applyCompatibilityMode() + { + if (isClientInitialized()) + // tools or clients should not limit the sstable formats they support + storageCompatibilityMode = StorageCompatibilityMode.NONE; + else if (conf != null && conf.storage_compatibility_mode != null) + storageCompatibilityMode = conf.storage_compatibility_mode; + } + private static void applySSTableFormats() { ServiceLoader loader = ServiceLoader.load(SSTableFormat.Factory.class, DatabaseDescriptor.class.getClassLoader()); @@ -4968,11 +4982,7 @@ public class DatabaseDescriptor public static StorageCompatibilityMode getStorageCompatibilityMode() { - // Config is null for junits that don't load the config. Get from env var that CI/build.xml sets - if (conf == null) - return CassandraRelevantProperties.JUNIT_STORAGE_COMPATIBILITY_MODE.getEnum(StorageCompatibilityMode.CASSANDRA_4); - else - return conf.storage_compatibility_mode; + return storageCompatibilityMode; } public static ParameterizedClass getDefaultCompaction() diff --git a/src/java/org/apache/cassandra/net/MessagingService.java b/src/java/org/apache/cassandra/net/MessagingService.java index d3c3d96565..903ef977b2 100644 --- a/src/java/org/apache/cassandra/net/MessagingService.java +++ b/src/java/org/apache/cassandra/net/MessagingService.java @@ -31,10 +31,10 @@ import java.util.stream.Collectors; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; -import io.netty.util.concurrent.Future; //checkstyle: permit this import import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import io.netty.util.concurrent.Future; //checkstyle: permit this import import org.apache.cassandra.concurrent.ScheduledExecutors; import org.apache.cassandra.concurrent.Stage; import org.apache.cassandra.config.DatabaseDescriptor; @@ -51,7 +51,6 @@ import org.apache.cassandra.utils.concurrent.FutureCombiner; import static java.util.Collections.synchronizedList; import static java.util.concurrent.TimeUnit.MINUTES; - import static org.apache.cassandra.concurrent.Stage.MUTATION; import static org.apache.cassandra.config.CassandraRelevantProperties.NON_GRACEFUL_SHUTDOWN; import static org.apache.cassandra.utils.Clock.Global.nanoTime; @@ -222,8 +221,6 @@ public class MessagingService extends MessagingServiceMBeanImpl implements Messa // c14227 TTL overflow, 'uint' timestamps VERSION_50(13); - public static final Version CURRENT = DatabaseDescriptor.getStorageCompatibilityMode().isBefore(5) ? VERSION_40 : VERSION_50; - public final int value; Version(int value) @@ -253,9 +250,26 @@ public class MessagingService extends MessagingServiceMBeanImpl implements Messa public static final int VERSION_40 = 12; public static final int VERSION_50 = 13; // c14227 TTL overflow, 'uint' timestamps public static final int minimum_version = VERSION_40; - public static final int current_version = Version.CURRENT.value; - static AcceptVersions accept_messaging = new AcceptVersions(minimum_version, current_version); - static AcceptVersions accept_streaming = new AcceptVersions(current_version, current_version); + public static final int maximum_version = VERSION_50; + // we want to use a modified behavior for the tools and clients - that is, since they are not running a server, they + // should not need to run in a compatibility mode. They should be able to connect to the server regardless whether + // it uses messaving version 4 or 5 + public static final int current_version = DatabaseDescriptor.getStorageCompatibilityMode().isBefore(5) ? VERSION_40 : VERSION_50; + static AcceptVersions accept_messaging; + static AcceptVersions accept_streaming; + static + { + if (DatabaseDescriptor.isClientInitialized()) + { + accept_messaging = new AcceptVersions(minimum_version, maximum_version); + accept_streaming = new AcceptVersions(minimum_version, maximum_version); + } + else + { + accept_messaging = new AcceptVersions(minimum_version, current_version); + accept_streaming = new AcceptVersions(current_version, current_version); + } + } static Map versionOrdinalMap = Arrays.stream(Version.values()).collect(Collectors.toMap(v -> v.value, v -> v.ordinal())); /** diff --git a/src/java/org/apache/cassandra/net/OutboundConnectionSettings.java b/src/java/org/apache/cassandra/net/OutboundConnectionSettings.java index 8a83a52aa6..ccc74f0aba 100644 --- a/src/java/org/apache/cassandra/net/OutboundConnectionSettings.java +++ b/src/java/org/apache/cassandra/net/OutboundConnectionSettings.java @@ -420,7 +420,7 @@ public class OutboundConnectionSettings if (tcpNoDelay != null) return tcpNoDelay; - if (isInLocalDC(getEndpointSnitch(), getBroadcastAddressAndPort(), to)) + if (DatabaseDescriptor.isClientOrToolInitialized() || isInLocalDC(getEndpointSnitch(), getBroadcastAddressAndPort(), to)) return INTRADC_TCP_NODELAY; return DatabaseDescriptor.getInterDCTcpNoDelay(); diff --git a/src/java/org/apache/cassandra/repair/messages/PrepareMessage.java b/src/java/org/apache/cassandra/repair/messages/PrepareMessage.java index bf76190fc3..a2cd7e3903 100644 --- a/src/java/org/apache/cassandra/repair/messages/PrepareMessage.java +++ b/src/java/org/apache/cassandra/repair/messages/PrepareMessage.java @@ -89,7 +89,9 @@ public class PrepareMessage extends RepairMessage } private static final String MIXED_MODE_ERROR = "Some nodes involved in repair are on an incompatible major version. " + - "Repair is not supported in mixed major version clusters."; + "Repair is not supported in mixed major version clusters. Note that " + + "5.x nodes running in storage compatibility mode = 4 are considered " + + "4.x nodes."; public static final IVersionedSerializer serializer = new IVersionedSerializer() { diff --git a/src/java/org/apache/cassandra/tools/BulkLoader.java b/src/java/org/apache/cassandra/tools/BulkLoader.java index 8802b9e837..d005f12746 100644 --- a/src/java/org/apache/cassandra/tools/BulkLoader.java +++ b/src/java/org/apache/cassandra/tools/BulkLoader.java @@ -60,7 +60,7 @@ public class BulkLoader public static void load(LoaderOptions options) throws BulkLoadException { - DatabaseDescriptor.toolInitialization(); + DatabaseDescriptor.clientInitialization(); OutputHandler handler = new OutputHandler.SystemOutput(options.verbose, options.debug); SSTableLoader loader = new SSTableLoader( options.directory.toAbsolute(), diff --git a/test/unit/org/apache/cassandra/repair/FuzzTestBase.java b/test/unit/org/apache/cassandra/repair/FuzzTestBase.java index fb7702cd82..c819cc465f 100644 --- a/test/unit/org/apache/cassandra/repair/FuzzTestBase.java +++ b/test/unit/org/apache/cassandra/repair/FuzzTestBase.java @@ -1204,7 +1204,7 @@ public abstract class FuzzTestBase extends CQLTester.InMemory { try (DataOutputBuffer b = DataOutputBuffer.scratchBuffer.get()) { - int messagingVersion = MessagingService.Version.CURRENT.value; + int messagingVersion = MessagingService.current_version; Message.serializer.serialize(msg, b, messagingVersion); DataInputBuffer in = new DataInputBuffer(b.unsafeGetBufferAndFlip(), false); return Message.serializer.deserialize(in, msg.from(), messagingVersion);