You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by bl...@apache.org on 2022/11/16 08:37:26 UTC
[cassandra] branch trunk updated (36e16ee3c9 -> 930f141fa0)
This is an automated email from the ASF dual-hosted git repository.
blambov pushed a change to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
from 36e16ee3c9 Adding endpoint verification option to client_encryption_options
new e08c7a6295 Avoid schema mismatch problems on memtable API misconfiguration
new 9f58d76f38 Avoid schema mismatch problems on memtable API misconfiguration
new 930f141fa0 Merge branch 'cassandra-4.1' into trunk
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/cassandra/db/memtable/Memtable_API.md | 7 +-
.../apache/cassandra/schema/MemtableParams.java | 18 ++++++
.../apache/cassandra/schema/SchemaKeyspace.java | 4 +-
.../cassandra/distributed/test/AlterTest.java | 75 ++++++++++++++++++++++
4 files changed, 100 insertions(+), 4 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
[cassandra] 02/02: Merge branch 'cassandra-4.1' into trunk
Posted by bl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
blambov pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
commit 930f141fa0f203df020d14903548e72aae2743f2
Merge: e08c7a6295 9f58d76f38
Author: Branimir Lambov <br...@datastax.com>
AuthorDate: Wed Nov 16 10:30:39 2022 +0200
Merge branch 'cassandra-4.1' into trunk
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
[cassandra] 01/02: Avoid schema mismatch problems on memtable API misconfiguration
Posted by bl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
blambov pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
commit e08c7a6295eed716e9973fbd809dfca32d68a3e8
Author: Branimir Lambov <br...@datastax.com>
AuthorDate: Mon Nov 14 13:59:05 2022 +0200
Avoid schema mismatch problems on memtable API misconfiguration
patch by Branimir Lambov; reviewed by Caleb Rackliffe for CASSANDRA-18040
---
.../apache/cassandra/db/memtable/Memtable_API.md | 7 +-
.../apache/cassandra/schema/MemtableParams.java | 18 ++++++
.../apache/cassandra/schema/SchemaKeyspace.java | 4 +-
.../cassandra/distributed/test/AlterTest.java | 75 ++++++++++++++++++++++
4 files changed, 100 insertions(+), 4 deletions(-)
diff --git a/src/java/org/apache/cassandra/db/memtable/Memtable_API.md b/src/java/org/apache/cassandra/db/memtable/Memtable_API.md
index e5399641c3..70f8f0b605 100644
--- a/src/java/org/apache/cassandra/db/memtable/Memtable_API.md
+++ b/src/java/org/apache/cassandra/db/memtable/Memtable_API.md
@@ -91,9 +91,10 @@ ALTER TABLE ... WITH memtable = 'default';
```
The memtable configuration selection is per table, i.e. it will be propagated to all nodes in the cluster. If some nodes
-do not have a definition for that configuration, cannot instantiate the class, or are still on a version of Cassandra
-before 4.1, they will reject the schema change. We therefore recommend using a separate `ALTER` statement to change a
-table's memtable implementation; upgrading all nodes to 4.1 or later is required to use the API.
+do not have a definition for that configuration or cannot instantiate the class, they will log an error and fall
+back to the default memtable configuration to avoid schema disagreements. However, if some nodes are still on a version
+of Cassandra before 4.1, they will reject the schema change. We therefore recommend using a separate `ALTER` statement
+to change a table's memtable implementation; upgrading all nodes to 4.1 or later is required to use the API.
As additional safety when first deploying an alternative implementation to a production cluster, one may consider
first deploying a remapped `default` configuration to all nodes in the cluster, switching the schema to reference
diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java
index a3f1bb2323..3470b7ac8d 100644
--- a/src/java/org/apache/cassandra/schema/MemtableParams.java
+++ b/src/java/org/apache/cassandra/schema/MemtableParams.java
@@ -28,6 +28,8 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableMap;
+import org.slf4j.LoggerFactory;
+
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.config.InheritingClass;
import org.apache.cassandra.config.ParameterizedClass;
@@ -110,6 +112,22 @@ public final class MemtableParams
}
}
+ public static MemtableParams getWithFallback(String key)
+ {
+ try
+ {
+ return get(key);
+ }
+ catch (ConfigurationException e)
+ {
+ LoggerFactory.getLogger(MemtableParams.class).error("Invalid memtable configuration \"" + key + "\" in schema. " +
+ "Falling back to default to avoid schema mismatch.\n" +
+ "Please ensure the correct definition is given in cassandra.yaml.",
+ e);
+ return new MemtableParams(DEFAULT.factory(), key);
+ }
+ }
+
@VisibleForTesting
static Map<String, ParameterizedClass> expandDefinitions(Map<String, InheritingClass> memtableConfigurations)
{
diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java
index cbcba70caa..0604fc1ffd 100644
--- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java
+++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java
@@ -967,7 +967,9 @@ public final class SchemaKeyspace
.comment(row.getString("comment"))
.compaction(CompactionParams.fromMap(row.getFrozenTextMap("compaction")))
.compression(CompressionParams.fromMap(row.getFrozenTextMap("compression")))
- .memtable(MemtableParams.get(row.has("memtable") ? row.getString("memtable") : null)) // memtable column was introduced in 4.1
+ .memtable(MemtableParams.getWithFallback(row.has("memtable")
+ ? row.getString("memtable")
+ : null)) // memtable column was introduced in 4.1
.defaultTimeToLive(row.getInt("default_time_to_live"))
.extensions(row.getFrozenMap("extensions", UTF8Type.instance, BytesType.instance))
.gcGraceSeconds(row.getInt("gc_grace_seconds"))
diff --git a/test/distributed/org/apache/cassandra/distributed/test/AlterTest.java b/test/distributed/org/apache/cassandra/distributed/test/AlterTest.java
index b8912b262f..907ba5e218 100644
--- a/test/distributed/org/apache/cassandra/distributed/test/AlterTest.java
+++ b/test/distributed/org/apache/cassandra/distributed/test/AlterTest.java
@@ -18,17 +18,28 @@
package org.apache.cassandra.distributed.test;
+import java.util.List;
+
+import com.google.common.collect.ImmutableMap;
import org.junit.Assert;
import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.cassandra.cql3.Lists;
import org.apache.cassandra.db.Keyspace;
import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.Feature;
import org.apache.cassandra.distributed.api.ICluster;
import org.apache.cassandra.distributed.api.IInstanceConfig;
import org.apache.cassandra.distributed.api.IInvokableInstance;
import org.apache.cassandra.distributed.api.IIsolatedExecutor;
import org.apache.cassandra.distributed.api.SimpleQueryResult;
+import org.apache.cassandra.distributed.api.TokenSupplier;
+import org.apache.cassandra.distributed.shared.ClusterUtils;
import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.utils.Throwables;
import static org.apache.cassandra.distributed.action.GossipHelper.withProperty;
import static org.apache.cassandra.distributed.api.ConsistencyLevel.ONE;
@@ -39,6 +50,7 @@ import static org.apache.cassandra.distributed.api.TokenSupplier.evenlyDistribut
import static org.apache.cassandra.distributed.shared.NetworkTopology.singleDcNetworkTopology;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.fail;
public class AlterTest extends TestBaseImpl
{
@@ -107,4 +119,67 @@ public class AlterTest extends TestBaseImpl
}
}
}
+
+ @Test
+ public void unknownMemtableConfigurationTest() throws Throwable
+ {
+ Logger logger = LoggerFactory.getLogger(getClass());
+ try (Cluster cluster = Cluster.build(1)
+ .withTokenSupplier(TokenSupplier.evenlyDistributedTokens(3, 1))
+ .withConfig(c -> c.with(Feature.values())
+ .set("memtable", ImmutableMap.of(
+ "configurations", ImmutableMap.of(
+ "testconfig", ImmutableMap.of(
+ "class_name", "SkipListMemtable")))))
+ .start())
+ {
+ init(cluster);
+ cluster.schemaChange("CREATE TABLE " + KEYSPACE + ".tbl (pk int PRIMARY KEY)");
+
+ // Start Node 2 without the memtable configuration definition.
+ IInvokableInstance node1 = cluster.get(1);
+ IInvokableInstance node2 = ClusterUtils.addInstance(cluster, node1.config(), c -> c.set("memtable", ImmutableMap.of()));
+ node2.startup(cluster);
+
+ try
+ {
+ cluster.schemaChange("ALTER TABLE " + KEYSPACE + ".tbl WITH memtable = 'testconfig'", false, node2);
+ fail("Expected ALTER to fail with unknown memtable configuration.");
+ }
+ catch (Throwable t)
+ {
+ // expected
+ logger.info("Expected: {}", t.getMessage());
+ Assert.assertTrue(Throwables.isCausedBy(t, x -> x.getMessage().matches("Memtable configuration.*not found.*")));
+ }
+ long mark = node2.logs().mark();
+
+ cluster.schemaChange("ALTER TABLE " + KEYSPACE + ".tbl WITH memtable = 'testconfig'", false, node1);
+ // the above should succeed, the configuration is acceptable to node1
+
+ ClusterUtils.awaitGossipSchemaMatch(cluster);
+ List<String> errorInLog = node2.logs().grep(mark,"ERROR.*Invalid memtable configuration.*").getResult();
+ Assert.assertTrue(errorInLog.size() > 0);
+ logger.info(Lists.listToString(errorInLog));
+
+ // Add a new node that has an invalid definition but should accept the already defined table schema.
+ IInvokableInstance node3 = ClusterUtils.addInstance(cluster,
+ node2.config(),
+ c -> c.set("memtable", ImmutableMap.of(
+ "configurations", ImmutableMap.of(
+ "testconfig", ImmutableMap.of(
+ "class_name", "NotExistingMemtable")))));
+ node3.startup(cluster);
+ ClusterUtils.awaitGossipSchemaMatch(cluster);
+
+ errorInLog = node3.logs().grep("ERROR.*Invalid memtable configuration.*").getResult();
+ Assert.assertTrue(errorInLog.size() > 0);
+ logger.info(Lists.listToString(errorInLog));
+
+ // verify that all nodes can write to the table
+ node1.executeInternalWithResult("INSERT INTO " + KEYSPACE + ".tbl (pk) VALUES (?)", 1);
+ node2.executeInternalWithResult("INSERT INTO " + KEYSPACE + ".tbl (pk) VALUES (?)", 2);
+ node3.executeInternalWithResult("INSERT INTO " + KEYSPACE + ".tbl (pk) VALUES (?)", 3);
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org