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 2021/04/17 00:05:20 UTC
[GitHub] [cassandra] ekaterinadimitrova2 commented on a change in pull request #967: CASSANDRA-16567 (trunk): Fix test ViewTest.testCompoundPartitionKey
ekaterinadimitrova2 commented on a change in pull request #967:
URL: https://github.com/apache/cassandra/pull/967#discussion_r615048451
##########
File path: src/java/org/apache/cassandra/db/view/TableViews.java
##########
@@ -93,6 +93,11 @@ public boolean add(View view)
return Iterables.transform(views, view -> keyspace.getColumnFamilyStore(view.getDefinition().name()));
}
+ public void stopBuild()
+ {
+ views.forEach(View::stopBuild);
Review comment:
Should we revise the debug log messages? WDYT?
https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/view/View.java#L223
##########
File path: test/unit/org/apache/cassandra/cql3/ViewTest.java
##########
@@ -50,11 +51,12 @@
import org.apache.cassandra.service.ClientWarn;
import org.apache.cassandra.transport.ProtocolVersion;
import org.apache.cassandra.utils.FBUtilities;
+import org.jboss.byteman.contrib.bmunit.BMRule;
+import org.jboss.byteman.contrib.bmunit.BMUnitRunner;
-
+@RunWith(BMUnitRunner.class)
public class ViewTest extends CQLTester
{
- ProtocolVersion protocolVersion = ProtocolVersion.V4;
Review comment:
Good catch, thanks for fixing it and making it impossible to be missed again.
##########
File path: test/unit/org/apache/cassandra/cql3/ViewTest.java
##########
@@ -1469,4 +1460,32 @@ public void testQuotedIdentifiersInWhereClause() throws Throwable
row("\"theKey\" IS NOT NULL AND (\"theClustering_1\", \"theClustering_2\") = (1, 2) AND \"theValue\" IS NOT NULL"),
row("token(\"theKey\") > token(1) AND \"theClustering_1\" = 1 AND \"theClustering_2\" > 2 AND \"theValue\" IS NOT NULL"));
}
+
+ /**
+ * Tests that truncating a table stops the ongoing builds of its materialized views,
+ * so they don't write into the MV data that has been truncated in the base table.
+ *
+ * See CASSANDRA-16567 for further details.
+ */
+ @Test
+ @BMRule(name = "Delay materialized view mutations",
+ targetClass = "StorageProxy",
+ targetMethod = "mutateMV",
+ action = "Thread.sleep(4000);")
+ public void testTruncateWhileBuilding() throws Throwable
+ {
+ createTable("CREATE TABLE %s (k int, c int, v int, PRIMARY KEY(k, c))");
+ execute("USE " + keyspace());
+ executeNet("USE " + keyspace());
+ updateView("INSERT INTO %s (k, c, v) VALUES (?, ?, ?)", 0, 0, 0);
Review comment:
updateView before createView?
##########
File path: test/unit/org/apache/cassandra/cql3/ViewTest.java
##########
@@ -1469,4 +1460,32 @@ public void testQuotedIdentifiersInWhereClause() throws Throwable
row("\"theKey\" IS NOT NULL AND (\"theClustering_1\", \"theClustering_2\") = (1, 2) AND \"theValue\" IS NOT NULL"),
row("token(\"theKey\") > token(1) AND \"theClustering_1\" = 1 AND \"theClustering_2\" > 2 AND \"theValue\" IS NOT NULL"));
}
+
+ /**
+ * Tests that truncating a table stops the ongoing builds of its materialized views,
+ * so they don't write into the MV data that has been truncated in the base table.
+ *
+ * See CASSANDRA-16567 for further details.
+ */
+ @Test
+ @BMRule(name = "Delay materialized view mutations",
+ targetClass = "StorageProxy",
+ targetMethod = "mutateMV",
+ action = "Thread.sleep(4000);")
+ public void testTruncateWhileBuilding() throws Throwable
+ {
+ createTable("CREATE TABLE %s (k int, c int, v int, PRIMARY KEY(k, c))");
+ execute("USE " + keyspace());
+ executeNet("USE " + keyspace());
+ updateView("INSERT INTO %s (k, c, v) VALUES (?, ?, ?)", 0, 0, 0);
+ createView("mv",
+ "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s " +
+ "WHERE k IS NOT NULL AND c IS NOT NULL AND v IS NOT NULL " +
+ "PRIMARY KEY (v, c, k)");
+ Thread.sleep(2000); // wait for the creation of MV build tasks
Review comment:
I hope these sleeps survive our CIs, I have bad feeling when we rely on timing but I guess it should be fine, you also pushed it on the multiplexer :-)
##########
File path: test/unit/org/apache/cassandra/cql3/ViewTest.java
##########
@@ -1469,4 +1460,32 @@ public void testQuotedIdentifiersInWhereClause() throws Throwable
row("\"theKey\" IS NOT NULL AND (\"theClustering_1\", \"theClustering_2\") = (1, 2) AND \"theValue\" IS NOT NULL"),
row("token(\"theKey\") > token(1) AND \"theClustering_1\" = 1 AND \"theClustering_2\" > 2 AND \"theValue\" IS NOT NULL"));
}
+
+ /**
+ * Tests that truncating a table stops the ongoing builds of its materialized views,
+ * so they don't write into the MV data that has been truncated in the base table.
+ *
+ * See CASSANDRA-16567 for further details.
+ */
+ @Test
+ @BMRule(name = "Delay materialized view mutations",
+ targetClass = "StorageProxy",
+ targetMethod = "mutateMV",
+ action = "Thread.sleep(4000);")
Review comment:
I guess 4000 came based on testing
##########
File path: test/unit/org/apache/cassandra/cql3/ViewTest.java
##########
@@ -1469,4 +1460,32 @@ public void testQuotedIdentifiersInWhereClause() throws Throwable
row("\"theKey\" IS NOT NULL AND (\"theClustering_1\", \"theClustering_2\") = (1, 2) AND \"theValue\" IS NOT NULL"),
row("token(\"theKey\") > token(1) AND \"theClustering_1\" = 1 AND \"theClustering_2\" > 2 AND \"theValue\" IS NOT NULL"));
}
+
+ /**
+ * Tests that truncating a table stops the ongoing builds of its materialized views,
+ * so they don't write into the MV data that has been truncated in the base table.
+ *
+ * See CASSANDRA-16567 for further details.
+ */
+ @Test
+ @BMRule(name = "Delay materialized view mutations",
+ targetClass = "StorageProxy",
+ targetMethod = "mutateMV",
+ action = "Thread.sleep(4000);")
+ public void testTruncateWhileBuilding() throws Throwable
+ {
+ createTable("CREATE TABLE %s (k int, c int, v int, PRIMARY KEY(k, c))");
+ execute("USE " + keyspace());
+ executeNet("USE " + keyspace());
+ updateView("INSERT INTO %s (k, c, v) VALUES (?, ?, ?)", 0, 0, 0);
+ createView("mv",
+ "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s " +
+ "WHERE k IS NOT NULL AND c IS NOT NULL AND v IS NOT NULL " +
+ "PRIMARY KEY (v, c, k)");
+ Thread.sleep(2000); // wait for the creation of MV build tasks
Review comment:
I would also probably add one more `assertRows(execute("SELECT * FROM mv WHERE v = ? and c = ?", 0, 0));` before `TRUNCATE`
--
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.
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