dcapwell commented on code in PR #3268:
URL: https://github.com/apache/cassandra/pull/3268#discussion_r1583584227
##########
src/java/org/apache/cassandra/concurrent/TaskFactory.java:
##########
@@ -90,6 +90,10 @@ public Runnable toExecute(WithResources withResources,
Runnable runnable)
@Override
public <T> RunnableFuture<T> toSubmit(WithResources withResources,
Runnable runnable)
{
+ if (runnable instanceof RunnableDebuggableTask)
+ return withResources.isNoOp() ? newTask(runnable)
+ : newTask(withResources,
runnable);
+
return withResources.isNoOp() ? newTask(callable(runnable))
: newTask(withResources,
callable(runnable));
}
Review Comment:
is there a reason to keep this logic and guard the above logic with `if
(runnable instanceof RunnableDebuggableTask)`? As far as I can tell there
isn't a good reason... we convert the runnable to `callable` for the execution
logic, and the debug logic deals with the raw type...
##########
test/distributed/org/apache/cassandra/distributed/test/QueriesTableTest.java:
##########
@@ -18,72 +18,152 @@
package org.apache.cassandra.distributed.test;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
+import java.io.IOException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
import org.junit.Test;
+import com.datastax.driver.core.Session;
import org.apache.cassandra.distributed.Cluster;
-import org.apache.cassandra.distributed.api.ConsistencyLevel;
+import org.apache.cassandra.distributed.api.Feature;
import org.apache.cassandra.distributed.api.Row;
import org.apache.cassandra.distributed.api.SimpleQueryResult;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
public class QueriesTableTest extends TestBaseImpl
{
public static final int ITERATIONS = 256;
+ private static Cluster SHARED_CLUSTER;
+ private static com.datastax.driver.core.Cluster DRIVER_CLUSTER;
+ private static Session SESSION;
+
+ @BeforeClass
+ public static void createCluster() throws IOException
+ {
+ SHARED_CLUSTER = init(Cluster.build(1).withConfig(c ->
c.with(Feature.NATIVE_PROTOCOL)).start());
+ DRIVER_CLUSTER =
com.datastax.driver.core.Cluster.builder().addContactPoint("127.0.0.1").build();
Review Comment:
FYI
`org.apache.cassandra.distributed.test.JavaDriverUtils#create(org.apache.cassandra.distributed.api.ICluster<?
extends org.apache.cassandra.distributed.api.IInstance>)`
does the same thing in this case.
##########
test/distributed/org/apache/cassandra/distributed/test/QueriesTableTest.java:
##########
@@ -18,72 +18,152 @@
package org.apache.cassandra.distributed.test;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
+import java.io.IOException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
import org.junit.Test;
+import com.datastax.driver.core.Session;
import org.apache.cassandra.distributed.Cluster;
-import org.apache.cassandra.distributed.api.ConsistencyLevel;
+import org.apache.cassandra.distributed.api.Feature;
import org.apache.cassandra.distributed.api.Row;
import org.apache.cassandra.distributed.api.SimpleQueryResult;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
public class QueriesTableTest extends TestBaseImpl
{
public static final int ITERATIONS = 256;
+ private static Cluster SHARED_CLUSTER;
+ private static com.datastax.driver.core.Cluster DRIVER_CLUSTER;
+ private static Session SESSION;
+
+ @BeforeClass
+ public static void createCluster() throws IOException
+ {
+ SHARED_CLUSTER = init(Cluster.build(1).withConfig(c ->
c.with(Feature.NATIVE_PROTOCOL)).start());
+ DRIVER_CLUSTER =
com.datastax.driver.core.Cluster.builder().addContactPoint("127.0.0.1").build();
+ SESSION = DRIVER_CLUSTER.connect();
+ }
+
+ @AfterClass
+ public static void closeCluster()
+ {
+ if (SESSION != null)
+ SESSION.close();
+
+ if (DRIVER_CLUSTER != null)
+ DRIVER_CLUSTER.close();
+
+ if (SHARED_CLUSTER != null)
+ SHARED_CLUSTER.close();
+ }
+
@Test
public void shouldExposeReadsAndWrites() throws Throwable
{
- try (Cluster cluster = init(Cluster.build(1).start()))
+ SHARED_CLUSTER.schemaChange("CREATE TABLE " + KEYSPACE + ".tbl (k int
primary key, v int)");
+
+ AtomicInteger reads = new AtomicInteger(0);
+ AtomicInteger coordinatorReads = new AtomicInteger(0);
+ AtomicInteger writes = new AtomicInteger(0);
+ AtomicInteger coordinatorWrites = new AtomicInteger(0);
+ int totalOperations = 0;
+
+ while (reads.get() == 0 || coordinatorReads.get() == 0 || writes.get()
== 0 || coordinatorWrites.get() == 0)
+ {
+ for (int i = 0; i < ITERATIONS; i++)
+ {
+ SESSION.executeAsync("INSERT INTO " + KEYSPACE + ".tbl (k, v)
VALUES (" + i + ", 0)");
+ SESSION.executeAsync("SELECT * FROM " + KEYSPACE + ".tbl WHERE
k = " + (i - 1));
+ }
+
+ totalOperations += ITERATIONS;
+
+ SimpleQueryResult result =
SHARED_CLUSTER.get(1).executeInternalWithResult("SELECT * FROM
system_views.queries");
Review Comment:
this is super racy and leaves you only validating that you saw reads/writes
and not that they were "correct". Might want to use BB to make this test
stable and avoid timing out (keep running over and over trying to find the last
query)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
