From 3cac8c6fa1ab575a798770c8581423f5743caeea Mon Sep 17 00:00:00 2001 From: CalvinKirs Date: Thu, 7 Jul 2022 11:15:39 +0800 Subject: [PATCH 1/6] Useing Awaitility for asynchroneous testing. --- bookkeeper-server/pom.xml | 4 ++ .../replication/AutoRecoveryMainTest.java | 55 ++++++------------- pom.xml | 7 +++ 3 files changed, 27 insertions(+), 39 deletions(-) diff --git a/bookkeeper-server/pom.xml b/bookkeeper-server/pom.xml index b4d1cfc474c..df14bd70698 100644 --- a/bookkeeper-server/pom.xml +++ b/bookkeeper-server/pom.xml @@ -209,6 +209,10 @@ ${project.parent.version} test + + org.awaitility + awaitility + diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java index f013399c3e4..1492bc29fd7 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java @@ -1,5 +1,4 @@ /** - * * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -7,23 +6,25 @@ * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

* Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * KIND, either express or implied. See the License for the * specific language governing permissions and limitations * under the License. - * */ package org.apache.bookkeeper.replication; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.awaitility.Awaitility.await; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import java.io.IOException; + import org.apache.bookkeeper.bookie.BookieImpl; import org.apache.bookkeeper.meta.zk.ZKMetadataClientDriver; import org.apache.bookkeeper.net.BookieId; @@ -49,11 +50,8 @@ public void testStartup() throws Exception { AutoRecoveryMain main = new AutoRecoveryMain(confByIndex(0)); try { main.start(); - Thread.sleep(500); - assertTrue("AuditorElector should be running", - main.auditorElector.isRunning()); - assertTrue("Replication worker should be running", - main.replicationWorker.isRunning()); + await().atMost(1,SECONDS).untilAsserted(() -> + assertTrue("AuditorElector and Replication Worker should be running",main.auditorElector.isRunning() && main.replicationWorker.isRunning())); } finally { main.shutdown(); } @@ -66,12 +64,8 @@ public void testStartup() throws Exception { public void testShutdown() throws Exception { AutoRecoveryMain main = new AutoRecoveryMain(confByIndex(0)); main.start(); - Thread.sleep(500); - assertTrue("AuditorElector should be running", - main.auditorElector.isRunning()); - assertTrue("Replication worker should be running", - main.replicationWorker.isRunning()); - + await().atMost(1,SECONDS).untilAsserted(()-> + assertTrue("AuditorElector and ReplicationWorker should be running",main.auditorElector.isRunning() && main.replicationWorker.isRunning())); main.shutdown(); assertFalse("AuditorElector should not be running", main.auditorElector.isRunning()); @@ -98,19 +92,7 @@ public void testAutoRecoverySessionLoss() throws Exception { */ ZKMetadataClientDriver zkMetadataClientDriver1 = startAutoRecoveryMain(main1); ZooKeeper zk1 = zkMetadataClientDriver1.getZk(); - - // Wait until auditor gets elected - for (int i = 0; i < 10; i++) { - try { - if (main1.auditorElector.getCurrentAuditor() != null) { - break; - } else { - Thread.sleep(1000); - } - } catch (IOException e) { - Thread.sleep(1000); - } - } + await().atMost(20, SECONDS).until(() -> main1.auditorElector.getAuditor() != null); BookieId currentAuditor = main1.auditorElector.getCurrentAuditor(); assertNotNull(currentAuditor); Auditor auditor1 = main1.auditorElector.getAuditor(); @@ -146,20 +128,15 @@ public void testAutoRecoverySessionLoss() throws Exception { * wait for some time for all the components of AR1 and AR2 are * shutdown. */ - for (int i = 0; i < 10; i++) { - if (!main1.auditorElector.isRunning() && !main1.replicationWorker.isRunning() - && !main1.isAutoRecoveryRunning() && !main2.auditorElector.isRunning() - && !main2.replicationWorker.isRunning() && !main2.isAutoRecoveryRunning()) { - break; - } - Thread.sleep(1000); - } + await().atMost(20,SECONDS).until(()->!main1.auditorElector.isRunning() && !main1.replicationWorker.isRunning() + && !main1.isAutoRecoveryRunning() && !main2.auditorElector.isRunning() + && !main2.replicationWorker.isRunning() && !main2.isAutoRecoveryRunning()); /* * the AR3 should be current auditor. */ currentAuditor = main3.auditorElector.getCurrentAuditor(); - assertTrue("Current Auditor should be AR3", currentAuditor.equals(BookieImpl.getBookieId(confByIndex(2)))); + assertEquals("Current Auditor should be AR3", currentAuditor, BookieImpl.getBookieId(confByIndex(2))); auditor3 = main3.auditorElector.getAuditor(); assertTrue("Auditor of AR3 should be running", auditor3.isRunning()); diff --git a/pom.xml b/pom.xml index dff71fdda7b..64184830b6a 100644 --- a/pom.xml +++ b/pom.xml @@ -209,6 +209,7 @@ 1 4.0.0 3.0.1 + 4.2.0 3.0.0-M6 @@ -839,6 +840,12 @@ powermock-module-junit4 test + + org.awaitility + awaitility + ${awaitility.version} + test + From dadd0abbfd4d2bd66135c491b9251175c1d68d5e Mon Sep 17 00:00:00 2001 From: CalvinKirs Date: Thu, 7 Jul 2022 11:43:08 +0800 Subject: [PATCH 2/6] Useing Awaitility for asynchroneous testing. --- .../bookkeeper/replication/AutoRecoveryMainTest.java | 8 +++++--- pom.xml | 6 ++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java index 1492bc29fd7..38a3ed9147b 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java @@ -1,4 +1,5 @@ /** + * * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -6,15 +7,16 @@ * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

+ * + * http://www.apache.org/licenses/LICENSE-2.0 + * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * KIND, either express or implied. See the License for the * specific language governing permissions and limitations * under the License. + * */ package org.apache.bookkeeper.replication; diff --git a/pom.xml b/pom.xml index 64184830b6a..ce7e69ba168 100644 --- a/pom.xml +++ b/pom.xml @@ -780,6 +780,12 @@ rxjava ${rxjava.version} + + org.awaitility + awaitility + ${awaitility.version} + test + From 42651a48818d0de79b82e2b45c46944245163532 Mon Sep 17 00:00:00 2001 From: CalvinKirs Date: Thu, 7 Jul 2022 13:03:12 +0800 Subject: [PATCH 3/6] fix deprecated method --- .../apache/bookkeeper/bookie/datainteg/WriteSetsTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/WriteSetsTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/WriteSetsTest.java index 1a82b0bde03..16350410949 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/WriteSetsTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/WriteSetsTest.java @@ -19,11 +19,12 @@ package org.apache.bookkeeper.bookie.datainteg; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.isIn; +import static org.hamcrest.Matchers.in; import com.google.common.collect.ImmutableList; @@ -168,7 +169,7 @@ private static void assertContentsMatch(ImmutableList writeSet, } for (int i = 0; i < distWriteSet.size(); i++) { - assertThat(distWriteSet.get(i), isIn(writeSet)); + assertThat(distWriteSet.get(i), is(in(writeSet))); } } } From 248eb3b7eef54240da515e61389900f909b36daa Mon Sep 17 00:00:00 2001 From: CalvinKirs Date: Thu, 7 Jul 2022 13:42:58 +0800 Subject: [PATCH 4/6] replace sleep in BookieZKExpireTest --- .../bookkeeper/replication/AutoRecoveryMainTest.java | 12 +++++++----- .../apache/bookkeeper/test/BookieZKExpireTest.java | 8 +++++--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java index 38a3ed9147b..70040e0c987 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java @@ -52,8 +52,9 @@ public void testStartup() throws Exception { AutoRecoveryMain main = new AutoRecoveryMain(confByIndex(0)); try { main.start(); - await().atMost(1,SECONDS).untilAsserted(() -> - assertTrue("AuditorElector and Replication Worker should be running",main.auditorElector.isRunning() && main.replicationWorker.isRunning())); + await().atMost(1, SECONDS).untilAsserted(() -> + assertTrue("AuditorElector and Replication Worker should be running", + main.auditorElector.isRunning() && main.replicationWorker.isRunning())); } finally { main.shutdown(); } @@ -66,8 +67,9 @@ public void testStartup() throws Exception { public void testShutdown() throws Exception { AutoRecoveryMain main = new AutoRecoveryMain(confByIndex(0)); main.start(); - await().atMost(1,SECONDS).untilAsserted(()-> - assertTrue("AuditorElector and ReplicationWorker should be running",main.auditorElector.isRunning() && main.replicationWorker.isRunning())); + await().atMost(1, SECONDS).untilAsserted(()-> + assertTrue("AuditorElector and ReplicationWorker should be running", + main.auditorElector.isRunning() && main.replicationWorker.isRunning())); main.shutdown(); assertFalse("AuditorElector should not be running", main.auditorElector.isRunning()); @@ -130,7 +132,7 @@ public void testAutoRecoverySessionLoss() throws Exception { * wait for some time for all the components of AR1 and AR2 are * shutdown. */ - await().atMost(20,SECONDS).until(()->!main1.auditorElector.isRunning() && !main1.replicationWorker.isRunning() + await().atMost(20, SECONDS).until(()->!main1.auditorElector.isRunning() && !main1.replicationWorker.isRunning() && !main1.isAutoRecoveryRunning() && !main2.auditorElector.isRunning() && !main2.replicationWorker.isRunning() && !main2.isAutoRecoveryRunning()); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java index 006c485eeee..b5ed6c97ce6 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java @@ -21,6 +21,7 @@ package org.apache.bookkeeper.test; +import static org.awaitility.Awaitility.await; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -29,6 +30,7 @@ import java.io.File; import java.util.HashSet; +import java.util.concurrent.TimeUnit; import org.apache.bookkeeper.bookie.MockUncleanShutdownDetection; import org.apache.bookkeeper.bookie.TestBookieImpl; @@ -76,7 +78,7 @@ conf, new TestBookieImpl(conf), int secondsToWait = 5; while (!server.isRunning()) { - Thread.sleep(1000); + await().atMost(1000, TimeUnit.MILLISECONDS); if (secondsToWait-- <= 0) { fail("Bookie never started"); } @@ -95,11 +97,11 @@ conf, new TestBookieImpl(conf), assertNotNull("Send thread not found", sendthread); sendthread.suspend(); - Thread.sleep(2 * conf.getZkTimeout()); + await().atMost(2L * conf.getZkTimeout(), TimeUnit.MILLISECONDS).await(); sendthread.resume(); // allow watcher thread to run - Thread.sleep(3000); + await().atMost(3, TimeUnit.SECONDS).await(); assertTrue("Bookie should not shutdown on losing zk session", server.isBookieRunning()); assertTrue("Bookie Server should not shutdown on losing zk session", server.isRunning()); } finally { From b4b7bf19791b3001f8b4c96ac9ce75c52bcd1cdb Mon Sep 17 00:00:00 2001 From: CalvinKirs Date: Sun, 10 Jul 2022 15:24:26 +0800 Subject: [PATCH 5/6] restore timeout parameter --- .../replication/AutoRecoveryMainTest.java | 4 +-- .../bookkeeper/test/BookieZKExpireTest.java | 28 +++++++------------ 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java index 70040e0c987..b05cc269273 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AutoRecoveryMainTest.java @@ -96,7 +96,7 @@ public void testAutoRecoverySessionLoss() throws Exception { */ ZKMetadataClientDriver zkMetadataClientDriver1 = startAutoRecoveryMain(main1); ZooKeeper zk1 = zkMetadataClientDriver1.getZk(); - await().atMost(20, SECONDS).until(() -> main1.auditorElector.getAuditor() != null); + await().until(() -> main1.auditorElector.getAuditor() != null); BookieId currentAuditor = main1.auditorElector.getCurrentAuditor(); assertNotNull(currentAuditor); Auditor auditor1 = main1.auditorElector.getAuditor(); @@ -132,7 +132,7 @@ public void testAutoRecoverySessionLoss() throws Exception { * wait for some time for all the components of AR1 and AR2 are * shutdown. */ - await().atMost(20, SECONDS).until(()->!main1.auditorElector.isRunning() && !main1.replicationWorker.isRunning() + await().until(()->!main1.auditorElector.isRunning() && !main1.replicationWorker.isRunning() && !main1.isAutoRecoveryRunning() && !main2.auditorElector.isRunning() && !main2.replicationWorker.isRunning() && !main2.isAutoRecoveryRunning()); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java index b5ed6c97ce6..b7b3c82d949 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java @@ -21,16 +21,15 @@ package org.apache.bookkeeper.test; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.awaitility.Awaitility.await; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import io.netty.buffer.UnpooledByteBufAllocator; import java.io.File; import java.util.HashSet; -import java.util.concurrent.TimeUnit; import org.apache.bookkeeper.bookie.MockUncleanShutdownDetection; import org.apache.bookkeeper.bookie.TestBookieImpl; @@ -75,35 +74,28 @@ conf, new TestBookieImpl(conf), NullStatsLogger.INSTANCE, UnpooledByteBufAllocator.DEFAULT, new MockUncleanShutdownDetection()); server.start(); - - int secondsToWait = 5; - while (!server.isRunning()) { - await().atMost(1000, TimeUnit.MILLISECONDS); - if (secondsToWait-- <= 0) { - fail("Bookie never started"); - } - } + + BookieServer finalServer = server; + await().atMost(5, SECONDS).until(finalServer::isRunning); Thread sendthread = null; threadCount = Thread.activeCount(); threads = new Thread[threadCount * 2]; threadCount = Thread.enumerate(threads); for (int i = 0; i < threadCount; i++) { - if (threads[i].getName().indexOf("SendThread") != -1 + if (threads[i].getName().contains("SendThread") && !threadset.contains(threads[i])) { sendthread = threads[i]; break; } } + assertNotNull("Send thread not found", sendthread); - - sendthread.suspend(); - await().atMost(2L * conf.getZkTimeout(), TimeUnit.MILLISECONDS).await(); + sendthread.suspend(); sendthread.resume(); - // allow watcher thread to run - await().atMost(3, TimeUnit.SECONDS).await(); - assertTrue("Bookie should not shutdown on losing zk session", server.isBookieRunning()); - assertTrue("Bookie Server should not shutdown on losing zk session", server.isRunning()); + await().atMost(5, SECONDS).untilAsserted(() -> + assertTrue("Bookie and Bookie Server should not shutdown on losing zk session", finalServer.isBookieRunning() && finalServer.isRunning())); + } finally { server.shutdown(); } From 51f94d15eae688c3c333a1470b23f7a5f78d337b Mon Sep 17 00:00:00 2001 From: CalvinKirs Date: Sun, 10 Jul 2022 15:29:59 +0800 Subject: [PATCH 6/6] fix checkstyle --- .../org/apache/bookkeeper/test/BookieZKExpireTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java index b7b3c82d949..8ad0319ec89 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java @@ -74,7 +74,6 @@ conf, new TestBookieImpl(conf), NullStatsLogger.INSTANCE, UnpooledByteBufAllocator.DEFAULT, new MockUncleanShutdownDetection()); server.start(); - BookieServer finalServer = server; await().atMost(5, SECONDS).until(finalServer::isRunning); Thread sendthread = null; @@ -88,14 +87,13 @@ conf, new TestBookieImpl(conf), break; } } - assertNotNull("Send thread not found", sendthread); - sendthread.suspend(); + sendthread.suspend(); sendthread.resume(); // allow watcher thread to run - await().atMost(5, SECONDS).untilAsserted(() -> - assertTrue("Bookie and Bookie Server should not shutdown on losing zk session", finalServer.isBookieRunning() && finalServer.isRunning())); - + await().atMost(5, SECONDS).untilAsserted(() -> + assertTrue("Bookie and Bookie Server should not shutdown on losing zk session", + finalServer.isBookieRunning() && finalServer.isRunning())); } finally { server.shutdown(); }