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:27 UTC
[cassandra] 01/02: Avoid schema mismatch problems on memtable API misconfiguration
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