You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/03/15 23:11:09 UTC

[GitHub] [kafka] hachikuji opened a new pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

hachikuji opened a new pull request #10323:
URL: https://github.com/apache/kafka/pull/10323


   This patch changes the raft simulation tests to use jqwik, which is a property testing library. This provides two main benefits:
   
   - It simplifies the randomization of test parameters. Currently the tests use a fixed set of `Random` seeds, which means that most builds are doing redundant work. We get a bigger benefit from allowing each build to test different parameterizations.
   - It makes it easier to reproduce failures. Whenever a test fails, jqwik will report the random seed that failed. A developer can then modify the `@Property` annotation to use that specific seed in order to reproduce the failure.
   
   Note that I have resisted making logical changes to the tests themselves. The only difference is the way the parameters are specified.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


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



[GitHub] [kafka] hachikuji commented on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-800468316


   I can't claim to have searched all the library options extensively. I found out about jqwik from @stanislavkozlovski. The documentation seemed clear and it took almost no time to get something working. I do like how extensible the framework seems. I thought I might need some of that here, but it turned out to be a pretty simple usage within the standard API.


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



[GitHub] [kafka] dajac commented on a change in pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
dajac commented on a change in pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#discussion_r594933714



##########
File path: raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java
##########
@@ -835,6 +734,16 @@ void poll() {
                 throw new RuntimeException("Uncaught exception during poll of node " + nodeId, e);
             }
         }
+
+        long highWatermark() {
+            Optional<LogOffsetMetadata> highWatermarkMetadata = client.quorum().highWatermark();
+            return highWatermarkMetadata.map(hw -> hw.offset).orElse(0L);

Review comment:
       nit: These two lines could be combined into one.




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



[GitHub] [kafka] hachikuji edited a comment on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji edited a comment on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-801542559


   I am planning to merge this shortly if there are no objections. I double-checked with @junrao about the note added in regard to the jqwik license in NOTICE.


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



[GitHub] [kafka] ijuma commented on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-800286314


   Nice! How did we pick `jqwik`?


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



[GitHub] [kafka] hachikuji edited a comment on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji edited a comment on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-801542559


   I am planning to merge this shortly if there are no objections. I double-checked with @junrao about the note added to NOTICE in regard to the jqwik license.


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



[GitHub] [kafka] ijuma commented on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-800480203


   @hachikuji Sounds good. Can we take a quick look at the two alternatives mentioned in the following blog post?
   https://blog.johanneslink.net/2020/01/28/why-is-pbt-not-popular-in-java/
   
   I think you choose the right one, but good to do a bit of due diligence as future Java-based property based tests will rely on whatever we pick here.


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



[GitHub] [kafka] hachikuji commented on a change in pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#discussion_r596423418



##########
File path: raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java
##########
@@ -70,335 +108,200 @@
     private static final int FETCH_MAX_WAIT_MS = 100;
     private static final int LINGER_MS = 0;
 
-    @Test
-    public void testInitialLeaderElectionQuorumSizeOne() {
-        testInitialLeaderElection(new QuorumConfig(1));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeTwo() {
-        testInitialLeaderElection(new QuorumConfig(2));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeThree() {
-        testInitialLeaderElection(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFour() {
-        testInitialLeaderElection(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFive() {
-        testInitialLeaderElection(new QuorumConfig(5));
-    }
-
-    private void testInitialLeaderElection(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThree() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThreeAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 1));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFour() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFourAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFive() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFiveAndThreeObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 3));
-    }
-
-    private void testElectionAfterLeaderFailure(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, false);
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThree() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFour() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFive() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderGracefulShutdown(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, true);
-    }
-
-    private void checkElectionAfterLeaderShutdown(QuorumConfig config, boolean isGracefulShutdown) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // Shutdown the leader and write some more data. We can verify the new leader has been elected
-            // by verifying that the high watermark can still advance.
-            int leaderId = cluster.latestLeader().getAsInt();
-            if (isGracefulShutdown) {
-                cluster.shutdown(leaderId);
-            } else {
-                cluster.kill(leaderId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // Restart the node and verify it catches up
-            cluster.start(leaderId);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeThree() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFour() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFive() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(5));
-    }
-
-    private void checkRecoveryAfterAllNodesFail(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // We kill all of the nodes. Then we bring back a majority and verify that
-            // they are able to elect a leader and continue making progress
-
-            cluster.killAll();
-
-            Iterator<Integer> nodeIdsIterator = cluster.nodes().iterator();
-            for (int i = 0; i < cluster.majoritySize(); i++) {
-                Integer nodeId = nodeIdsIterator.next();
-                cluster.start(nodeId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThree() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFour() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFive() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderNetworkPartition(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 2);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // The leader gets partitioned off. We can verify the new leader has been elected
-            // by writing some data and ensuring that it gets replicated
-            int leaderId = cluster.latestLeader().getAsInt();
-            router.filter(leaderId, new DropAllTraffic());
-
-            Set<Integer> nonPartitionedNodes = new HashSet<>(cluster.nodes());
-            nonPartitionedNodes.remove(leaderId);
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20, nonPartitionedNodes));
-        }
+    @Property(tries = 100)

Review comment:
       The PR is reducing the number of runs by quite a lot. I justified this on account of the randomized seed which allow more of the builds to be doing useful (i.e. non-redundant) work. I think there are a bunch of improvements we can make to improve execution time overall. Also I suspect there was a regression since these used to execute much faster. Really I'd like to get to the point where we can run thousands of simulations on each build without significantly adding build time.




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



[GitHub] [kafka] jsancio commented on a change in pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
jsancio commented on a change in pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#discussion_r596449459



##########
File path: raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java
##########
@@ -70,335 +108,200 @@
     private static final int FETCH_MAX_WAIT_MS = 100;
     private static final int LINGER_MS = 0;
 
-    @Test
-    public void testInitialLeaderElectionQuorumSizeOne() {
-        testInitialLeaderElection(new QuorumConfig(1));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeTwo() {
-        testInitialLeaderElection(new QuorumConfig(2));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeThree() {
-        testInitialLeaderElection(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFour() {
-        testInitialLeaderElection(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFive() {
-        testInitialLeaderElection(new QuorumConfig(5));
-    }
-
-    private void testInitialLeaderElection(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThree() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThreeAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 1));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFour() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFourAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFive() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFiveAndThreeObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 3));
-    }
-
-    private void testElectionAfterLeaderFailure(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, false);
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThree() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFour() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFive() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderGracefulShutdown(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, true);
-    }
-
-    private void checkElectionAfterLeaderShutdown(QuorumConfig config, boolean isGracefulShutdown) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // Shutdown the leader and write some more data. We can verify the new leader has been elected
-            // by verifying that the high watermark can still advance.
-            int leaderId = cluster.latestLeader().getAsInt();
-            if (isGracefulShutdown) {
-                cluster.shutdown(leaderId);
-            } else {
-                cluster.kill(leaderId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // Restart the node and verify it catches up
-            cluster.start(leaderId);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeThree() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFour() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFive() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(5));
-    }
-
-    private void checkRecoveryAfterAllNodesFail(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // We kill all of the nodes. Then we bring back a majority and verify that
-            // they are able to elect a leader and continue making progress
-
-            cluster.killAll();
-
-            Iterator<Integer> nodeIdsIterator = cluster.nodes().iterator();
-            for (int i = 0; i < cluster.majoritySize(); i++) {
-                Integer nodeId = nodeIdsIterator.next();
-                cluster.start(nodeId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThree() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFour() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFive() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderNetworkPartition(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 2);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // The leader gets partitioned off. We can verify the new leader has been elected
-            // by writing some data and ensuring that it gets replicated
-            int leaderId = cluster.latestLeader().getAsInt();
-            router.filter(leaderId, new DropAllTraffic());
-
-            Set<Integer> nonPartitionedNodes = new HashSet<>(cluster.nodes());
-            nonPartitionedNodes.remove(leaderId);
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20, nonPartitionedNodes));
-        }
+    @Property(tries = 100)

Review comment:
       Very cool. I made the same change to `KafkaMetadataLog`: https://github.com/apache/kafka/pull/10344




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



[GitHub] [kafka] ijuma edited a comment on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
ijuma edited a comment on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-800535044


   Thanks, this is very helpful. Since this is EPL 2, we need to check https://apache.org/legal/resolved.html#weak-copyleft-licenses


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



[GitHub] [kafka] hachikuji merged pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji merged pull request #10323:
URL: https://github.com/apache/kafka/pull/10323


   


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



[GitHub] [kafka] hachikuji commented on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-801325094


   @chia7712 I removed `forRandomRunning` here: https://github.com/apache/kafka/pull/10309.


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



[GitHub] [kafka] chia7712 commented on a change in pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#discussion_r594972521



##########
File path: raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java
##########
@@ -525,33 +430,27 @@ void schedule(Runnable action, int delayMs, int periodMs, int jitterMs) {
         }
 
         void runUntil(Supplier<Boolean> exitCondition) {
-            while (!exitCondition.get()) {
-                if (queue.isEmpty())
+            for (int iteration = 0; iteration < MAX_ITERATIONS; iteration++) {

Review comment:
       How about setting `@Timeout` instead of `MAX_ITERATIONS`?




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



[GitHub] [kafka] chia7712 commented on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-801321480


   unrelated to this PR. `forRandomRunning` is not used. Maybe we can remove it in this PR? Also, `random` of `RaftNode` is not used also.


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



[GitHub] [kafka] hachikuji commented on a change in pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#discussion_r596432176



##########
File path: raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java
##########
@@ -70,335 +108,200 @@
     private static final int FETCH_MAX_WAIT_MS = 100;
     private static final int LINGER_MS = 0;
 
-    @Test
-    public void testInitialLeaderElectionQuorumSizeOne() {
-        testInitialLeaderElection(new QuorumConfig(1));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeTwo() {
-        testInitialLeaderElection(new QuorumConfig(2));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeThree() {
-        testInitialLeaderElection(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFour() {
-        testInitialLeaderElection(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFive() {
-        testInitialLeaderElection(new QuorumConfig(5));
-    }
-
-    private void testInitialLeaderElection(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThree() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThreeAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 1));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFour() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFourAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFive() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFiveAndThreeObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 3));
-    }
-
-    private void testElectionAfterLeaderFailure(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, false);
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThree() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFour() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFive() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderGracefulShutdown(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, true);
-    }
-
-    private void checkElectionAfterLeaderShutdown(QuorumConfig config, boolean isGracefulShutdown) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // Shutdown the leader and write some more data. We can verify the new leader has been elected
-            // by verifying that the high watermark can still advance.
-            int leaderId = cluster.latestLeader().getAsInt();
-            if (isGracefulShutdown) {
-                cluster.shutdown(leaderId);
-            } else {
-                cluster.kill(leaderId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // Restart the node and verify it catches up
-            cluster.start(leaderId);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeThree() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFour() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFive() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(5));
-    }
-
-    private void checkRecoveryAfterAllNodesFail(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // We kill all of the nodes. Then we bring back a majority and verify that
-            // they are able to elect a leader and continue making progress
-
-            cluster.killAll();
-
-            Iterator<Integer> nodeIdsIterator = cluster.nodes().iterator();
-            for (int i = 0; i < cluster.majoritySize(); i++) {
-                Integer nodeId = nodeIdsIterator.next();
-                cluster.start(nodeId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThree() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFour() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFive() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderNetworkPartition(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 2);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // The leader gets partitioned off. We can verify the new leader has been elected
-            // by writing some data and ensuring that it gets replicated
-            int leaderId = cluster.latestLeader().getAsInt();
-            router.filter(leaderId, new DropAllTraffic());
-
-            Set<Integer> nonPartitionedNodes = new HashSet<>(cluster.nodes());
-            nonPartitionedNodes.remove(leaderId);
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20, nonPartitionedNodes));
-        }
+    @Property(tries = 100)

Review comment:
       I was inspired to take a look and found the big regression. The way we catch NoSuchElementException in `MockLog` was killing the performance of these tests because of need to fill in the stack trace. I tested locally with a fix and it brought the local time for this test from over 4 minutes to 48 seconds. On trunk, this test is taking more than 16 minutes for me, so overall this patch will be a major improvement.




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



[GitHub] [kafka] hachikuji commented on a change in pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#discussion_r596419808



##########
File path: raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java
##########
@@ -70,335 +108,200 @@
     private static final int FETCH_MAX_WAIT_MS = 100;
     private static final int LINGER_MS = 0;
 
-    @Test
-    public void testInitialLeaderElectionQuorumSizeOne() {
-        testInitialLeaderElection(new QuorumConfig(1));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeTwo() {
-        testInitialLeaderElection(new QuorumConfig(2));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeThree() {
-        testInitialLeaderElection(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFour() {
-        testInitialLeaderElection(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFive() {
-        testInitialLeaderElection(new QuorumConfig(5));
-    }
-
-    private void testInitialLeaderElection(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThree() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThreeAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 1));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFour() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFourAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFive() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFiveAndThreeObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 3));
-    }
-
-    private void testElectionAfterLeaderFailure(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, false);
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThree() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFour() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFive() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderGracefulShutdown(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, true);
-    }
-
-    private void checkElectionAfterLeaderShutdown(QuorumConfig config, boolean isGracefulShutdown) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // Shutdown the leader and write some more data. We can verify the new leader has been elected
-            // by verifying that the high watermark can still advance.
-            int leaderId = cluster.latestLeader().getAsInt();
-            if (isGracefulShutdown) {
-                cluster.shutdown(leaderId);
-            } else {
-                cluster.kill(leaderId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // Restart the node and verify it catches up
-            cluster.start(leaderId);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeThree() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFour() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFive() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(5));
-    }
-
-    private void checkRecoveryAfterAllNodesFail(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // We kill all of the nodes. Then we bring back a majority and verify that
-            // they are able to elect a leader and continue making progress
-
-            cluster.killAll();
-
-            Iterator<Integer> nodeIdsIterator = cluster.nodes().iterator();
-            for (int i = 0; i < cluster.majoritySize(); i++) {
-                Integer nodeId = nodeIdsIterator.next();
-                cluster.start(nodeId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThree() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFour() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFive() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderNetworkPartition(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 2);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // The leader gets partitioned off. We can verify the new leader has been elected
-            // by writing some data and ensuring that it gets replicated
-            int leaderId = cluster.latestLeader().getAsInt();
-            router.filter(leaderId, new DropAllTraffic());
-
-            Set<Integer> nonPartitionedNodes = new HashSet<>(cluster.nodes());
-            nonPartitionedNodes.remove(leaderId);
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20, nonPartitionedNodes));
-        }
+    @Property(tries = 100)
+    void canElectInitialLeader(
+        @ForAll Random random,

Review comment:
       Probably, but here want it to run with multiple random seeds.

##########
File path: raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java
##########
@@ -70,335 +108,200 @@
     private static final int FETCH_MAX_WAIT_MS = 100;
     private static final int LINGER_MS = 0;
 
-    @Test
-    public void testInitialLeaderElectionQuorumSizeOne() {
-        testInitialLeaderElection(new QuorumConfig(1));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeTwo() {
-        testInitialLeaderElection(new QuorumConfig(2));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeThree() {
-        testInitialLeaderElection(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFour() {
-        testInitialLeaderElection(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFive() {
-        testInitialLeaderElection(new QuorumConfig(5));
-    }
-
-    private void testInitialLeaderElection(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThree() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThreeAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 1));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFour() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFourAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFive() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFiveAndThreeObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 3));
-    }
-
-    private void testElectionAfterLeaderFailure(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, false);
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThree() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFour() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFive() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderGracefulShutdown(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, true);
-    }
-
-    private void checkElectionAfterLeaderShutdown(QuorumConfig config, boolean isGracefulShutdown) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // Shutdown the leader and write some more data. We can verify the new leader has been elected
-            // by verifying that the high watermark can still advance.
-            int leaderId = cluster.latestLeader().getAsInt();
-            if (isGracefulShutdown) {
-                cluster.shutdown(leaderId);
-            } else {
-                cluster.kill(leaderId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // Restart the node and verify it catches up
-            cluster.start(leaderId);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeThree() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFour() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFive() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(5));
-    }
-
-    private void checkRecoveryAfterAllNodesFail(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // We kill all of the nodes. Then we bring back a majority and verify that
-            // they are able to elect a leader and continue making progress
-
-            cluster.killAll();
-
-            Iterator<Integer> nodeIdsIterator = cluster.nodes().iterator();
-            for (int i = 0; i < cluster.majoritySize(); i++) {
-                Integer nodeId = nodeIdsIterator.next();
-                cluster.start(nodeId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThree() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFour() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFive() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderNetworkPartition(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 2);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // The leader gets partitioned off. We can verify the new leader has been elected
-            // by writing some data and ensuring that it gets replicated
-            int leaderId = cluster.latestLeader().getAsInt();
-            router.filter(leaderId, new DropAllTraffic());
-
-            Set<Integer> nonPartitionedNodes = new HashSet<>(cluster.nodes());
-            nonPartitionedNodes.remove(leaderId);
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20, nonPartitionedNodes));
-        }
+    @Property(tries = 100)
+    void canElectInitialLeader(
+        @ForAll Random random,

Review comment:
       Probably, but here I want it to run with multiple random seeds.




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



[GitHub] [kafka] jsancio commented on a change in pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
jsancio commented on a change in pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#discussion_r596410292



##########
File path: raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java
##########
@@ -70,335 +108,200 @@
     private static final int FETCH_MAX_WAIT_MS = 100;
     private static final int LINGER_MS = 0;
 
-    @Test
-    public void testInitialLeaderElectionQuorumSizeOne() {
-        testInitialLeaderElection(new QuorumConfig(1));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeTwo() {
-        testInitialLeaderElection(new QuorumConfig(2));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeThree() {
-        testInitialLeaderElection(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFour() {
-        testInitialLeaderElection(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFive() {
-        testInitialLeaderElection(new QuorumConfig(5));
-    }
-
-    private void testInitialLeaderElection(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThree() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThreeAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 1));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFour() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFourAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFive() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFiveAndThreeObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 3));
-    }
-
-    private void testElectionAfterLeaderFailure(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, false);
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThree() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFour() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFive() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderGracefulShutdown(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, true);
-    }
-
-    private void checkElectionAfterLeaderShutdown(QuorumConfig config, boolean isGracefulShutdown) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // Shutdown the leader and write some more data. We can verify the new leader has been elected
-            // by verifying that the high watermark can still advance.
-            int leaderId = cluster.latestLeader().getAsInt();
-            if (isGracefulShutdown) {
-                cluster.shutdown(leaderId);
-            } else {
-                cluster.kill(leaderId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // Restart the node and verify it catches up
-            cluster.start(leaderId);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeThree() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFour() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFive() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(5));
-    }
-
-    private void checkRecoveryAfterAllNodesFail(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // We kill all of the nodes. Then we bring back a majority and verify that
-            // they are able to elect a leader and continue making progress
-
-            cluster.killAll();
-
-            Iterator<Integer> nodeIdsIterator = cluster.nodes().iterator();
-            for (int i = 0; i < cluster.majoritySize(); i++) {
-                Integer nodeId = nodeIdsIterator.next();
-                cluster.start(nodeId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThree() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFour() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFive() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderNetworkPartition(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 2);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // The leader gets partitioned off. We can verify the new leader has been elected
-            // by writing some data and ensuring that it gets replicated
-            int leaderId = cluster.latestLeader().getAsInt();
-            router.filter(leaderId, new DropAllTraffic());
-
-            Set<Integer> nonPartitionedNodes = new HashSet<>(cluster.nodes());
-            nonPartitionedNodes.remove(leaderId);
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20, nonPartitionedNodes));
-        }
+    @Property(tries = 100)
+    void canElectInitialLeader(
+        @ForAll Random random,

Review comment:
       I assume that `@ForAll` will cause the library to run this test multiple times with different random seeds. If so is there a way to tell the library to only run it once for some random seed?

##########
File path: raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java
##########
@@ -70,335 +108,200 @@
     private static final int FETCH_MAX_WAIT_MS = 100;
     private static final int LINGER_MS = 0;
 
-    @Test
-    public void testInitialLeaderElectionQuorumSizeOne() {
-        testInitialLeaderElection(new QuorumConfig(1));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeTwo() {
-        testInitialLeaderElection(new QuorumConfig(2));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeThree() {
-        testInitialLeaderElection(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFour() {
-        testInitialLeaderElection(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testInitialLeaderElectionQuorumSizeFive() {
-        testInitialLeaderElection(new QuorumConfig(5));
-    }
-
-    private void testInitialLeaderElection(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThree() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeThreeAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(3, 1));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFour() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFourAndTwoObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFive() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderFailureQuorumSizeFiveAndThreeObservers() {
-        testElectionAfterLeaderFailure(new QuorumConfig(5, 3));
-    }
-
-    private void testElectionAfterLeaderFailure(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, false);
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThree() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFour() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFive() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 0));
-    }
-
-    @Test
-    public void testElectionAfterLeaderGracefulShutdownQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderGracefulShutdown(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderGracefulShutdown(QuorumConfig config) {
-        checkElectionAfterLeaderShutdown(config, true);
-    }
-
-    private void checkElectionAfterLeaderShutdown(QuorumConfig config, boolean isGracefulShutdown) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // Shutdown the leader and write some more data. We can verify the new leader has been elected
-            // by verifying that the high watermark can still advance.
-            int leaderId = cluster.latestLeader().getAsInt();
-            if (isGracefulShutdown) {
-                cluster.shutdown(leaderId);
-            } else {
-                cluster.kill(leaderId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // Restart the node and verify it catches up
-            cluster.start(leaderId);
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeThree() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFour() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testRecoveryAfterAllNodesFailQuorumSizeFive() {
-        checkRecoveryAfterAllNodesFail(new QuorumConfig(5));
-    }
-
-    private void checkRecoveryAfterAllNodesFail(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 1);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-            long highWatermark = cluster.maxHighWatermarkReached();
-
-            // We kill all of the nodes. Then we bring back a majority and verify that
-            // they are able to elect a leader and continue making progress
-
-            cluster.killAll();
-
-            Iterator<Integer> nodeIdsIterator = cluster.nodes().iterator();
-            for (int i = 0; i < cluster.majoritySize(); i++) {
-                Integer nodeId = nodeIdsIterator.next();
-                cluster.start(nodeId);
-            }
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(highWatermark + 10));
-        }
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThree() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeThreeAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(3, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFour() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFourAndTwoObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(4, 2));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFive() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5));
-    }
-
-    @Test
-    public void testElectionAfterLeaderNetworkPartitionQuorumSizeFiveAndThreeObservers() {
-        checkElectionAfterLeaderNetworkPartition(new QuorumConfig(5, 3));
-    }
-
-    private void checkElectionAfterLeaderNetworkPartition(QuorumConfig config) {
-        for (int seed = 0; seed < 100; seed++) {
-            Cluster cluster = new Cluster(config, seed);
-            MessageRouter router = new MessageRouter(cluster);
-            EventScheduler scheduler = schedulerWithDefaultInvariants(cluster);
-
-            // Seed the cluster with some data
-            cluster.startAll();
-            schedulePolling(scheduler, cluster, 3, 5);
-            scheduler.schedule(router::deliverAll, 0, 2, 2);
-            scheduler.schedule(new SequentialAppendAction(cluster), 0, 2, 3);
-            scheduler.runUntil(cluster::hasConsistentLeader);
-            scheduler.runUntil(() -> cluster.anyReachedHighWatermark(10));
-
-            // The leader gets partitioned off. We can verify the new leader has been elected
-            // by writing some data and ensuring that it gets replicated
-            int leaderId = cluster.latestLeader().getAsInt();
-            router.filter(leaderId, new DropAllTraffic());
-
-            Set<Integer> nonPartitionedNodes = new HashSet<>(cluster.nodes());
-            nonPartitionedNodes.remove(leaderId);
-
-            scheduler.runUntil(() -> cluster.allReachedHighWatermark(20, nonPartitionedNodes));
-        }
+    @Property(tries = 100)

Review comment:
       Hmm. These simulation tests are already the slowest suite in `raft:test`. I `git fetch` the PR and it looks like they still run when calling `./gradlew raft:test`. Is there away to set this to a smaller number (2) as the default and override it if the user wants to run them with 100 tries?
   
   Btw, `5` (number of possible voters) * `6` (number of possible observers) is `30` which is much smaller than `100`. I assume that the library will run the same configuration with multiple seeds.




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



[GitHub] [kafka] stanislavkozlovski commented on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
stanislavkozlovski commented on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-800488343


   For what it's worth I evaluated jqwik vs Quickcheck and had the following bullet-point summaries:
   
   ### Quickcheck
   - 819 stars
   - 10-year old library
   - v1.0 released Nov 22, 2020
   - 25 open issues
   - 55 commits in the last year
   - MIT License
   - light documentation
   - supports shrinking
   
   ### Jqwik
   - 260 stars
   - 5-year old library
   - v1.5 release Feb 2021
   - 16 open issues
   - 1039 commits in the last year
   - EPL-2.0 License
   - extensive documentation
   - very configurable
   - supports shrinking
   
   My reasoning to prefer Jqwik was that it seemed more actively maintained, had good interfaces, had very extensive documentation (I value this heavily) and most importantly supports programmatic parameter generation, meaning it allows you to easily express the dependencies of randomized input. I got the notion that this random input dependency generation is one of the trickier things when writing more complex test cases from [this blog post](https://www.leadingagile.com/2018/04/step-by-step-toward-property-based-testing/).
   
   Jqwik has some other interesting features like [collecting and reporting statstics](https://jqwik.net/docs/current/user-guide.html#collecting-and-reporting-statistics) on the data it generates, allowing you to inspect what the generated data is and whether it's useful or can be tweaked.


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



[GitHub] [kafka] ijuma commented on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-800535044


   Since this is EPL 2, we need to check https://apache.org/legal/resolved.html#weak-copyleft-licenses


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



[GitHub] [kafka] hachikuji commented on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-801227767


   @ijuma Thanks for the link. EPL 2.0 is listed as a "weak copyleft" license, similar to Jersey under CDDL. The document says that we can include a binary dependence as long as it is appropriately labeled. I've updated the NOTICE file with language similar to the note about Jersey.


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



[GitHub] [kafka] hachikuji commented on pull request #10323: KAFKA-12459; Use property testing library for raft event simulation tests

Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #10323:
URL: https://github.com/apache/kafka/pull/10323#issuecomment-801542559


   I am planning to merge this shortly if there are no objections. I double-checked with @junrao about the note added about the jqwik license in NOTICE.


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