You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2022/11/14 12:16:59 UTC
[GitHub] [cassandra] blambov opened a new pull request, #2010: Avoid schema mismatch problems on memtable API misconfiguration, 4.1 branch
blambov opened a new pull request, #2010:
URL: https://github.com/apache/cassandra/pull/2010
Falls back to default on schema change carrying invalid memtable configuration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] maedhroz commented on a diff in pull request #2010: Avoid schema mismatch problems on memtable API misconfiguration, 4.1 branch
Posted by GitBox <gi...@apache.org>.
maedhroz commented on code in PR #2010:
URL: https://github.com/apache/cassandra/pull/2010#discussion_r1023271070
##########
test/distributed/org/apache/cassandra/distributed/test/AlterTest.java:
##########
@@ -107,4 +120,73 @@ public void alteringKeyspaceOnInsufficientNumberOfReplicasFiltersOutGossppingOnl
}
}
}
+
+ @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();
Review Comment:
nit: `mark` unused?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] maedhroz commented on a diff in pull request #2010: Avoid schema mismatch problems on memtable API misconfiguration, 4.1 branch
Posted by GitBox <gi...@apache.org>.
maedhroz commented on code in PR #2010:
URL: https://github.com/apache/cassandra/pull/2010#discussion_r1023287934
##########
src/java/org/apache/cassandra/schema/MemtableParams.java:
##########
@@ -110,6 +112,22 @@ public static MemtableParams get(String key)
}
}
+ public static MemtableParams getWithFallback(String key)
+ {
+ try
+ {
+ return get(key);
+ }
+ catch (ConfigurationException e)
+ {
+ LoggerFactory.getLogger(MemtableParams.class).error("Invalid memtable configuration \"" + key + "\" in schema. " +
Review Comment:
nit: Any reason we create the logger inline? (Looks like it would be the only usage, but just curious...)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blambov commented on a diff in pull request #2010: Avoid schema mismatch problems on memtable API misconfiguration, 4.1 branch
Posted by GitBox <gi...@apache.org>.
blambov commented on code in PR #2010:
URL: https://github.com/apache/cassandra/pull/2010#discussion_r1023683167
##########
src/java/org/apache/cassandra/schema/MemtableParams.java:
##########
@@ -110,6 +112,22 @@ public static MemtableParams get(String key)
}
}
+ public static MemtableParams getWithFallback(String key)
+ {
+ try
+ {
+ return get(key);
+ }
+ catch (ConfigurationException e)
+ {
+ LoggerFactory.getLogger(MemtableParams.class).error("Invalid memtable configuration \"" + key + "\" in schema. " +
Review Comment:
It's never used in other places, and this is a rare event which to me did not warrant always creating the logger object on its own.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blambov commented on a diff in pull request #2010: Avoid schema mismatch problems on memtable API misconfiguration, 4.1 branch
Posted by GitBox <gi...@apache.org>.
blambov commented on code in PR #2010:
URL: https://github.com/apache/cassandra/pull/2010#discussion_r1023681649
##########
test/distributed/org/apache/cassandra/distributed/test/AlterTest.java:
##########
@@ -107,4 +120,73 @@ public void alteringKeyspaceOnInsufficientNumberOfReplicasFiltersOutGossppingOnl
}
}
}
+
+ @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();
Review Comment:
Fixed now, `mark` is used is finding the relevant error after this point in the logs below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blambov closed pull request #2010: Avoid schema mismatch problems on memtable API misconfiguration, 4.1 branch
Posted by GitBox <gi...@apache.org>.
blambov closed pull request #2010: Avoid schema mismatch problems on memtable API misconfiguration, 4.1 branch
URL: https://github.com/apache/cassandra/pull/2010
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org