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/07/04 09:51:39 UTC

[GitHub] [cassandra] adelapena commented on a diff in pull request #1714: CASSANDRA-17307 4.1: Flaky MixedModeAvailability tests

adelapena commented on code in PR #1714:
URL: https://github.com/apache/cassandra/pull/1714#discussion_r912832176


##########
test/distributed/org/apache/cassandra/distributed/upgrade/MixedModeAvailabilityTestBase.java:
##########
@@ -18,77 +18,129 @@
 
 package org.apache.cassandra.distributed.upgrade;
 
-import java.util.Arrays;
-import java.util.List;
 import java.util.UUID;
+import java.util.concurrent.RejectedExecutionException;
 
-import com.vdurmont.semver4j.Semver;
+import org.junit.Test;
 
+import com.vdurmont.semver4j.Semver;
 import org.apache.cassandra.distributed.api.ConsistencyLevel;
 import org.apache.cassandra.distributed.api.ICoordinator;
+import org.apache.cassandra.exceptions.ReadFailureException;
 import org.apache.cassandra.exceptions.ReadTimeoutException;
+import org.apache.cassandra.exceptions.WriteFailureException;
 import org.apache.cassandra.exceptions.WriteTimeoutException;
 import org.apache.cassandra.net.Verb;
+import org.assertj.core.api.Assertions;
 
+import static java.lang.String.format;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.cassandra.distributed.api.ConsistencyLevel.ALL;
 import static org.apache.cassandra.distributed.api.ConsistencyLevel.ONE;
 import static org.apache.cassandra.distributed.api.ConsistencyLevel.QUORUM;
 import static org.apache.cassandra.distributed.shared.AssertUtils.assertRows;
 import static org.apache.cassandra.distributed.shared.AssertUtils.row;
 import static org.apache.cassandra.net.Verb.READ_REQ;
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static java.lang.String.format;
 
 
-public class MixedModeAvailabilityTestBase extends UpgradeTestBase
+public abstract class MixedModeAvailabilityTestBase extends UpgradeTestBase
 {
     private static final int NUM_NODES = 3;
     private static final int COORDINATOR = 1;
-    private static final List<Tester> TESTERS = Arrays.asList(new Tester(ONE, ALL),
-                                                              new Tester(QUORUM, QUORUM),
-                                                              new Tester(ALL, ONE));
+    private static final String INSERT = withKeyspace("INSERT INTO %s.t (k, c, v) VALUES (?, ?, ?)");
+    private static final String SELECT = withKeyspace("SELECT * FROM %s.t WHERE k = ?");
 
+    private final Semver initial;
+    private final ConsistencyLevel writeConsistencyLevel;
+    private final ConsistencyLevel readConsistencyLevel;
+
+    public MixedModeAvailabilityTestBase(Semver initial, ConsistencyLevel writeConsistencyLevel, ConsistencyLevel readConsistencyLevel)
+    {
+        this.initial = initial;
+        this.writeConsistencyLevel = writeConsistencyLevel;
+        this.readConsistencyLevel = readConsistencyLevel;
+    }
 
-    protected static void testAvailability(Semver initial) throws Throwable
+    @Test
+    public void testAvailabilityCoordinatorNotUpgraded() throws Throwable
     {
-        testAvailability(initial, UpgradeTestBase.CURRENT);
+        testAvailability(false, initial, writeConsistencyLevel, readConsistencyLevel);
     }
 
-    protected static void testAvailability(Semver initial, Semver upgrade) throws Throwable
+    @Test
+    public void testAvailabilityCoordinatorUpgraded() throws Throwable
     {
-        testAvailability(true, initial, upgrade);
-        testAvailability(false, initial, upgrade);
+        testAvailability(true, initial, writeConsistencyLevel, readConsistencyLevel);
     }
 
     private static void testAvailability(boolean upgradedCoordinator,
                                          Semver initial,
-                                         Semver upgrade) throws Throwable
+                                         ConsistencyLevel writeConsistencyLevel,
+                                         ConsistencyLevel readConsistencyLevel) throws Throwable
     {
         new TestCase()
         .nodes(NUM_NODES)
         .nodesToUpgrade(upgradedCoordinator ? 1 : 2)
-        .upgrades(initial, upgrade)
-        .withConfig(config -> config.set("read_request_timeout_in_ms", SECONDS.toMillis(2))
-                                    .set("write_request_timeout_in_ms", SECONDS.toMillis(2)))
-        .setup(c -> c.schemaChange(withKeyspace("CREATE TABLE %s.t (k uuid, c int, v int, PRIMARY KEY (k, c))")))
+        .upgrades(initial, UpgradeTestBase.CURRENT)
+        .withConfig(config -> config.set("read_request_timeout_in_ms", SECONDS.toMillis(5))
+                                    .set("write_request_timeout_in_ms", SECONDS.toMillis(5)))
+        // use retry of 10ms so that each check is consistent
+        // At the start of the world cfs.sampleLatencyNanos == 0, which means speculation acts as if ALWAYS is done,
+        // but after the first refresh this gets set high enough that we don't trigger speculation for the rest of the test!
+        // To be consistent set retry to 10ms so cfs.sampleLatencyNanos stays consistent for the duration of the test.
+        .setup(cluster -> {
+            cluster.schemaChange(withKeyspace("CREATE TABLE %s.t (k uuid, c int, v int, PRIMARY KEY (k, c)) WITH speculative_retry = '10ms'"));
+            cluster.setUncaughtExceptionsFilter(throwable -> throwable instanceof RejectedExecutionException);
+        })
         .runAfterNodeUpgrade((cluster, n) -> {
 
+            ICoordinator coordinator = cluster.coordinator(COORDINATOR);
+
             // using 0 to 2 down nodes...
-            for (int numNodesDown = 0; numNodesDown < NUM_NODES; numNodesDown++)
+            for (int i = 0; i < NUM_NODES; i++)
             {
+                final int numNodesDown = i;

Review Comment:
   We need a `final` copy of the non-final `i`, so we can use it in the lambda that we pass to the second call to `maybeFail`, [here](https://github.com/apache/cassandra/blob/f3e082990e00cc54b9e24212c2ab6da8f937344d/test/distributed/org/apache/cassandra/distributed/upgrade/MixedModeAvailabilityTestBase.java#L130).



-- 
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