This is an automated email from the ASF dual-hosted git repository.
blambov pushed a commit to branch cassandra-4.1
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-4.1 by this push:
new 9f58d76f38 Avoid schema mismatch problems on memtable API
misconfiguration
9f58d76f38 is described below
commit 9f58d76f3841864be11f5b9c4534027451328569
Author: Branimir Lambov <branimir.lam...@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 | 82 ++++++++++++++++++++++
4 files changed, 107 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 39f9b201af..8a582e3ad9 100644
--- a/src/java/org/apache/cassandra/db/memtable/Memtable_API.md
+++ b/src/java/org/apache/cassandra/db/memtable/Memtable_API.md
@@ -72,9 +72,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 33d2b7d6c8..b4f27830ef 100644
--- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java
+++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java
@@ -960,7 +960,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..2061c29141 100644
--- a/test/distributed/org/apache/cassandra/distributed/test/AlterTest.java
+++ b/test/distributed/org/apache/cassandra/distributed/test/AlterTest.java
@@ -18,17 +18,29 @@
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.distributed.util.QueryResultUtil;
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 +51,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 +120,73 @@ 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
+
+ final String schema1 =
QueryResultUtil.expand(node1.executeInternalWithResult("SELECT * FROM
system_schema.tables WHERE keyspace_name=?", KEYSPACE));
+ final String schema2 =
QueryResultUtil.expand(node2.executeInternalWithResult("SELECT * FROM
system_schema.tables WHERE keyspace_name=?", KEYSPACE));
+ logger.info("node1 schema: \n{}", schema1);
+ logger.info("node2 schema: \n{}", schema2);
+ Assert.assertEquals(schema1, schema2);
+ 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);
+ final String schema3 =
QueryResultUtil.expand(node3.executeInternalWithResult("SELECT * FROM
system_schema.tables WHERE keyspace_name=?", KEYSPACE));
+ logger.info("node3 schema: \n{}", schema3);
+ Assert.assertEquals(schema1, schema3);
+
+ 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-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
