You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/02/16 16:29:36 UTC
[bookkeeper] branch master updated: Fix auditor shutdown logic and
move decommision tests out of BookKeeperAdminTest
This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new c43d8c4 Fix auditor shutdown logic and move decommision tests out of BookKeeperAdminTest
c43d8c4 is described below
commit c43d8c49fe212a5253c2dfc27c3c3b8c018c697a
Author: Sijie Guo <si...@apache.org>
AuthorDate: Sat Feb 17 00:29:23 2018 +0800
Fix auditor shutdown logic and move decommision tests out of BookKeeperAdminTest
Descriptions of the changes in this PR:
- the auditor shutdown logic is problematic. most of the tests can finish quickly however it spend more 30 seconds on shutting down.
because the shutdown logic will be blocked until `awaitTermination` timed out.
- most of the tests in BookKeeperAdminTest don't need 6 bookies. so move the decommission tests to a separate class.
Author: Sijie Guo <si...@apache.org>
Reviewers: Ivan Kelly <iv...@apache.org>, Charan Reddy Guttapalem <re...@gmail.com>, Jia Zhai <None>
This closes #1099 from sijie/improve_admin_tests
---
.../org/apache/bookkeeper/replication/Auditor.java | 3 +-
.../bookkeeper/client/BookKeeperAdminTest.java | 153 ++----------------
.../bookkeeper/client/BookieDecommissionTest.java | 176 +++++++++++++++++++++
3 files changed, 191 insertions(+), 141 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
index 188ce71..9e4858a 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
@@ -707,8 +707,7 @@ public class Auditor {
*/
public void shutdown() {
LOG.info("Shutting down auditor");
- submitShutdownTask();
-
+ executor.shutdown();
try {
while (!executor.awaitTermination(30, TimeUnit.SECONDS)) {
LOG.warn("Executor not shutting down, interrupting");
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperAdminTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperAdminTest.java
index e1cb252..4112592 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperAdminTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperAdminTest.java
@@ -31,16 +31,13 @@ import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Random;
-
import org.apache.bookkeeper.bookie.Bookie;
-import org.apache.bookkeeper.client.BKException.BKIllegalOpException;
import org.apache.bookkeeper.client.BookKeeper.DigestType;
import org.apache.bookkeeper.conf.ClientConfiguration;
import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager;
import org.apache.bookkeeper.replication.ReplicationException.UnavailableException;
import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
-import org.apache.bookkeeper.test.annotations.FlakyTest;
import org.apache.bookkeeper.util.BookKeeperConstants;
import org.apache.commons.io.FileUtils;
import org.apache.zookeeper.CreateMode;
@@ -58,7 +55,7 @@ public class BookKeeperAdminTest extends BookKeeperClusterTestCase {
private static final Logger LOG = LoggerFactory.getLogger(BookKeeperAdminTest.class);
private DigestType digestType = DigestType.CRC32;
private static final String PASSWORD = "testPasswd";
- private static final int numOfBookies = 6;
+ private static final int numOfBookies = 2;
private final int lostBookieRecoveryDelayInitValue = 1800;
public BookKeeperAdminTest() {
@@ -70,16 +67,19 @@ public class BookKeeperAdminTest extends BookKeeperClusterTestCase {
@Test
public void testLostBookieRecoveryDelayValue() throws Exception {
- BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString());
- assertEquals("LostBookieRecoveryDelay", lostBookieRecoveryDelayInitValue, bkAdmin.getLostBookieRecoveryDelay());
- int newLostBookieRecoveryDelayValue = 2400;
- bkAdmin.setLostBookieRecoveryDelay(newLostBookieRecoveryDelayValue);
- assertEquals("LostBookieRecoveryDelay", newLostBookieRecoveryDelayValue, bkAdmin.getLostBookieRecoveryDelay());
- assertEquals("LostBookieRecoveryDelay", newLostBookieRecoveryDelayValue, bkAdmin.getLostBookieRecoveryDelay());
- newLostBookieRecoveryDelayValue = 3000;
- bkAdmin.setLostBookieRecoveryDelay(newLostBookieRecoveryDelayValue);
- assertEquals("LostBookieRecoveryDelay", newLostBookieRecoveryDelayValue, bkAdmin.getLostBookieRecoveryDelay());
- bkAdmin.close();
+ try (BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString())) {
+ assertEquals("LostBookieRecoveryDelay",
+ lostBookieRecoveryDelayInitValue, bkAdmin.getLostBookieRecoveryDelay());
+ int newLostBookieRecoveryDelayValue = 2400;
+ bkAdmin.setLostBookieRecoveryDelay(newLostBookieRecoveryDelayValue);
+ assertEquals("LostBookieRecoveryDelay",
+ newLostBookieRecoveryDelayValue, bkAdmin.getLostBookieRecoveryDelay());
+ newLostBookieRecoveryDelayValue = 3000;
+ bkAdmin.setLostBookieRecoveryDelay(newLostBookieRecoveryDelayValue);
+ assertEquals("LostBookieRecoveryDelay",
+ newLostBookieRecoveryDelayValue, bkAdmin.getLostBookieRecoveryDelay());
+ LOG.info("Test Done");
+ }
}
@Test
@@ -118,131 +118,6 @@ public class BookKeeperAdminTest extends BookKeeperClusterTestCase {
bkAdmin.close();
}
- @FlakyTest("https://github.com/apache/bookkeeper/issues/502")
- public void testDecommissionBookie() throws Exception {
- ZkLedgerUnderreplicationManager urLedgerMgr = new ZkLedgerUnderreplicationManager(baseClientConf, zkc);
- BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString());
-
- int numOfLedgers = 2 * numOfBookies;
- int numOfEntries = 2 * numOfBookies;
- for (int i = 0; i < numOfLedgers; i++) {
- LedgerHandle lh = bkc.createLedger(3, 2, digestType, PASSWORD.getBytes());
- for (int j = 0; j < numOfEntries; j++) {
- lh.addEntry("entry".getBytes());
- }
- lh.close();
- }
- /*
- * create ledgers having empty segments (segment with no entries)
- */
- for (int i = 0; i < numOfLedgers; i++) {
- LedgerHandle emptylh = bkc.createLedger(3, 2, digestType, PASSWORD.getBytes());
- emptylh.close();
- }
-
- try {
- /*
- * if we try to call decommissionBookie for a bookie which is not
- * shutdown, then it should throw BKIllegalOpException
- */
- bkAdmin.decommissionBookie(bs.get(0).getLocalAddress());
- fail("Expected BKIllegalOpException because that bookie is not shutdown yet");
- } catch (BKIllegalOpException bkioexc) {
- // expected IllegalException
- }
-
- ServerConfiguration killedBookieConf = killBookie(1);
- /*
- * this decommisionBookie should make sure that there are no
- * underreplicated ledgers because of this bookie
- */
- bkAdmin.decommissionBookie(Bookie.getBookieAddress(killedBookieConf));
- bkAdmin.triggerAudit();
- Thread.sleep(500);
- Iterator<Long> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
- if (ledgersToRereplicate.hasNext()) {
- while (ledgersToRereplicate.hasNext()) {
- Long ledgerId = ledgersToRereplicate.next();
- LOG.error("Ledger: {} is underreplicated which is not expected", ledgerId);
- }
- fail("There are not supposed to be any underreplicatedledgers");
- }
-
- killedBookieConf = killBookie(0);
- bkAdmin.decommissionBookie(Bookie.getBookieAddress(killedBookieConf));
- bkAdmin.triggerAudit();
- Thread.sleep(500);
- ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
- if (ledgersToRereplicate.hasNext()) {
- while (ledgersToRereplicate.hasNext()) {
- Long ledgerId = ledgersToRereplicate.next();
- LOG.error("Ledger: {} is underreplicated which is not expected", ledgerId);
- }
- fail("There are not supposed to be any underreplicatedledgers");
- }
- bkAdmin.close();
- }
-
- @Test
- public void testDecommissionForLedgersWithMultipleSegmentsAndNotWriteClosed() throws Exception {
- ZkLedgerUnderreplicationManager urLedgerMgr = new ZkLedgerUnderreplicationManager(baseClientConf, zkc);
- BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString());
- int numOfEntries = 2 * numOfBookies;
-
- LedgerHandle lh1 = bkc.createLedgerAdv(1L, numBookies, 3, 3, digestType, PASSWORD.getBytes(), null);
- LedgerHandle lh2 = bkc.createLedgerAdv(2L, numBookies, 3, 3, digestType, PASSWORD.getBytes(), null);
- LedgerHandle lh3 = bkc.createLedgerAdv(3L, numBookies, 3, 3, digestType, PASSWORD.getBytes(), null);
- LedgerHandle lh4 = bkc.createLedgerAdv(4L, numBookies, 3, 3, digestType, PASSWORD.getBytes(), null);
- for (int j = 0; j < numOfEntries; j++) {
- lh1.addEntry(j, "data".getBytes());
- lh2.addEntry(j, "data".getBytes());
- lh3.addEntry(j, "data".getBytes());
- lh4.addEntry(j, "data".getBytes());
- }
-
- startNewBookie();
-
- assertEquals("Number of Available Bookies", numOfBookies + 1, bkAdmin.getAvailableBookies().size());
-
- ServerConfiguration killedBookieConf = killBookie(0);
-
- /*
- * since one of the bookie is killed, ensemble change happens when next
- * write is made.So new fragment will be created for those 2 ledgers.
- */
- for (int j = numOfEntries; j < 2 * numOfEntries; j++) {
- lh1.addEntry(j, "data".getBytes());
- lh2.addEntry(j, "data".getBytes());
- }
-
- /*
- * Here lh1 and lh2 have multiple fragments and are writeclosed. But lh3 and lh4 are
- * not writeclosed and contains only one fragment.
- */
- lh1.close();
- lh2.close();
-
- /*
- * If the last fragment of the ledger is underreplicated and if the
- * ledger is not closed then it will remain underreplicated for
- * openLedgerRereplicationGracePeriod (by default 30 secs). For more
- * info. Check BOOKKEEPER-237 and BOOKKEEPER-325. But later
- * ReplicationWorker will fence the ledger.
- */
- bkAdmin.decommissionBookie(Bookie.getBookieAddress(killedBookieConf));
- bkAdmin.triggerAudit();
- Thread.sleep(500);
- Iterator<Long> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
- if (ledgersToRereplicate.hasNext()) {
- while (ledgersToRereplicate.hasNext()) {
- Long ledgerId = ledgersToRereplicate.next();
- LOG.error("Ledger: {} is underreplicated which is not expected", ledgerId);
- }
- fail("There are not supposed to be any underreplicatedledgers");
- }
- bkAdmin.close();
- }
-
@Test
public void testBookieInit() throws Exception {
int bookieindex = 0;
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieDecommissionTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieDecommissionTest.java
new file mode 100644
index 0000000..04b66a4
--- /dev/null
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieDecommissionTest.java
@@ -0,0 +1,176 @@
+/*
+ * 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
+ * regarding copyright ownership. The ASF licenses this file
+ * 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
+ *
+ * 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.client;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.util.Iterator;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.bookkeeper.bookie.Bookie;
+import org.apache.bookkeeper.client.BKException.BKIllegalOpException;
+import org.apache.bookkeeper.client.BookKeeper.DigestType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager;
+import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
+import org.apache.bookkeeper.test.annotations.FlakyTest;
+import org.junit.Test;
+
+/**
+ * Unit test of bookie decommission operations.
+ */
+@Slf4j
+public class BookieDecommissionTest extends BookKeeperClusterTestCase {
+
+ private static final int NUM_OF_BOOKIES = 6;
+ private static DigestType digestType = DigestType.CRC32;
+ private static final String PASSWORD = "testPasswd";
+
+ public BookieDecommissionTest() {
+ super(NUM_OF_BOOKIES, 480);
+ baseConf.setOpenLedgerRereplicationGracePeriod(String.valueOf(30000));
+ setAutoRecoveryEnabled(true);
+ }
+
+ @FlakyTest("https://github.com/apache/bookkeeper/issues/502")
+ public void testDecommissionBookie() throws Exception {
+ ZkLedgerUnderreplicationManager urLedgerMgr = new ZkLedgerUnderreplicationManager(baseClientConf, zkc);
+ BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString());
+
+ int numOfLedgers = 2 * NUM_OF_BOOKIES;
+ int numOfEntries = 2 * NUM_OF_BOOKIES;
+ for (int i = 0; i < numOfLedgers; i++) {
+ LedgerHandle lh = bkc.createLedger(3, 2, digestType, PASSWORD.getBytes());
+ for (int j = 0; j < numOfEntries; j++) {
+ lh.addEntry("entry".getBytes());
+ }
+ lh.close();
+ }
+ /*
+ * create ledgers having empty segments (segment with no entries)
+ */
+ for (int i = 0; i < numOfLedgers; i++) {
+ LedgerHandle emptylh = bkc.createLedger(3, 2, digestType, PASSWORD.getBytes());
+ emptylh.close();
+ }
+
+ try {
+ /*
+ * if we try to call decommissionBookie for a bookie which is not
+ * shutdown, then it should throw BKIllegalOpException
+ */
+ bkAdmin.decommissionBookie(bs.get(0).getLocalAddress());
+ fail("Expected BKIllegalOpException because that bookie is not shutdown yet");
+ } catch (BKIllegalOpException bkioexc) {
+ // expected IllegalException
+ }
+
+ ServerConfiguration killedBookieConf = killBookie(1);
+ /*
+ * this decommisionBookie should make sure that there are no
+ * underreplicated ledgers because of this bookie
+ */
+ bkAdmin.decommissionBookie(Bookie.getBookieAddress(killedBookieConf));
+ bkAdmin.triggerAudit();
+ Thread.sleep(500);
+ Iterator<Long> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
+ if (ledgersToRereplicate.hasNext()) {
+ while (ledgersToRereplicate.hasNext()) {
+ Long ledgerId = ledgersToRereplicate.next();
+ log.error("Ledger: {} is underreplicated which is not expected", ledgerId);
+ }
+ fail("There are not supposed to be any underreplicatedledgers");
+ }
+
+ killedBookieConf = killBookie(0);
+ bkAdmin.decommissionBookie(Bookie.getBookieAddress(killedBookieConf));
+ bkAdmin.triggerAudit();
+ Thread.sleep(500);
+ ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
+ if (ledgersToRereplicate.hasNext()) {
+ while (ledgersToRereplicate.hasNext()) {
+ Long ledgerId = ledgersToRereplicate.next();
+ log.error("Ledger: {} is underreplicated which is not expected", ledgerId);
+ }
+ fail("There are not supposed to be any underreplicatedledgers");
+ }
+ bkAdmin.close();
+ }
+
+ @Test
+ public void testDecommissionForLedgersWithMultipleSegmentsAndNotWriteClosed() throws Exception {
+ ZkLedgerUnderreplicationManager urLedgerMgr = new ZkLedgerUnderreplicationManager(baseClientConf, zkc);
+ BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString());
+ int numOfEntries = 2 * NUM_OF_BOOKIES;
+
+ LedgerHandle lh1 = bkc.createLedgerAdv(1L, numBookies, 3, 3, digestType, PASSWORD.getBytes(), null);
+ LedgerHandle lh2 = bkc.createLedgerAdv(2L, numBookies, 3, 3, digestType, PASSWORD.getBytes(), null);
+ LedgerHandle lh3 = bkc.createLedgerAdv(3L, numBookies, 3, 3, digestType, PASSWORD.getBytes(), null);
+ LedgerHandle lh4 = bkc.createLedgerAdv(4L, numBookies, 3, 3, digestType, PASSWORD.getBytes(), null);
+ for (int j = 0; j < numOfEntries; j++) {
+ lh1.addEntry(j, "data".getBytes());
+ lh2.addEntry(j, "data".getBytes());
+ lh3.addEntry(j, "data".getBytes());
+ lh4.addEntry(j, "data".getBytes());
+ }
+
+ startNewBookie();
+
+ assertEquals("Number of Available Bookies", NUM_OF_BOOKIES + 1, bkAdmin.getAvailableBookies().size());
+
+ ServerConfiguration killedBookieConf = killBookie(0);
+
+ /*
+ * since one of the bookie is killed, ensemble change happens when next
+ * write is made.So new fragment will be created for those 2 ledgers.
+ */
+ for (int j = numOfEntries; j < 2 * numOfEntries; j++) {
+ lh1.addEntry(j, "data".getBytes());
+ lh2.addEntry(j, "data".getBytes());
+ }
+
+ /*
+ * Here lh1 and lh2 have multiple fragments and are writeclosed. But lh3 and lh4 are
+ * not writeclosed and contains only one fragment.
+ */
+ lh1.close();
+ lh2.close();
+
+ /*
+ * If the last fragment of the ledger is underreplicated and if the
+ * ledger is not closed then it will remain underreplicated for
+ * openLedgerRereplicationGracePeriod (by default 30 secs). For more
+ * info. Check BOOKKEEPER-237 and BOOKKEEPER-325. But later
+ * ReplicationWorker will fence the ledger.
+ */
+ bkAdmin.decommissionBookie(Bookie.getBookieAddress(killedBookieConf));
+ bkAdmin.triggerAudit();
+ Thread.sleep(500);
+ Iterator<Long> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
+ if (ledgersToRereplicate.hasNext()) {
+ while (ledgersToRereplicate.hasNext()) {
+ Long ledgerId = ledgersToRereplicate.next();
+ log.error("Ledger: {} is underreplicated which is not expected", ledgerId);
+ }
+ fail("There are not supposed to be any underreplicatedledgers");
+ }
+ bkAdmin.close();
+ }
+
+}
--
To stop receiving notification emails like this one, please contact
sijie@apache.org.