You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by yo...@apache.org on 2022/08/01 13:44:54 UTC
[bookkeeper] 08/17: Autorecovery to rereplicate empty ledgers (#3239)
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit b715cf77b9ad1d8ece828a7799384124331147a3
Author: Andrey Yegorov <86...@users.noreply.github.com>
AuthorDate: Tue Jun 21 02:44:38 2022 -0700
Autorecovery to rereplicate empty ledgers (#3239)
(cherry picked from commit eadbdd4b6bfeef9924a3ff2c59fc3718cf3dc06b)
---
.../apache/bookkeeper/client/BookKeeperAdmin.java | 2 +-
.../apache/bookkeeper/client/LedgerChecker.java | 9 +-
.../bookkeeper/client/BookieDecommissionTest.java | 107 +++++++++++++++++++--
.../bookkeeper/client/TestLedgerChecker.java | 24 ++---
.../replication/BookieAutoRecoveryTest.java | 1 +
.../bookkeeper/test/BookKeeperClusterTestCase.java | 22 ++---
6 files changed, 132 insertions(+), 33 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
index cb3057a503..54cb566a30 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
@@ -1649,7 +1649,7 @@ public class BookKeeperAdmin implements AutoCloseable {
LOG.debug("Ledger: {} has been deleted", ledgerId);
return false;
} else {
- LOG.error("Got exception while trying to read LedgerMeatadata of " + ledgerId, e);
+ LOG.error("Got exception while trying to read LedgerMetadata of " + ledgerId, e);
throw new RuntimeException(e);
}
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java
index 87cbd5fa72..f3a49d31a6 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java
@@ -40,7 +40,6 @@ import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.ReadEntryCallback
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
/**
* A utility class to check the complete ledger and finds the UnderReplicated fragments if any.
*
@@ -238,7 +237,13 @@ public class LedgerChecker {
if (lastStored != LedgerHandle.INVALID_ENTRY_ID) {
throw new InvalidFragmentException();
}
- cb.operationComplete(BKException.Code.OK, fragment);
+
+ if (bookieWatcher.isBookieUnavailable(fragment.getAddress(bookieIndex))) {
+ // fragment is on this bookie, but already know it's unavailable, so skip the call
+ cb.operationComplete(BKException.Code.BookieHandleNotAvailableException, fragment);
+ } else {
+ cb.operationComplete(BKException.Code.OK, fragment);
+ }
} else if (bookieWatcher.isBookieUnavailable(fragment.getAddress(bookieIndex))) {
// fragment is on this bookie, but already know it's unavailable, so skip the call
cb.operationComplete(BKException.Code.BookieHandleNotAvailableException, fragment);
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
index d395ddf771..00127a2801 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieDecommissionTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieDecommissionTest.java
@@ -19,8 +19,11 @@
package org.apache.bookkeeper.client;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.fail;
import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
import lombok.extern.slf4j.Slf4j;
import org.apache.bookkeeper.bookie.BookieImpl;
import org.apache.bookkeeper.client.BKException.BKIllegalOpException;
@@ -29,6 +32,7 @@ import org.apache.bookkeeper.common.testing.annotations.FlakyTest;
import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.meta.UnderreplicatedLedger;
import org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager;
+import org.apache.bookkeeper.net.BookieId;
import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
import org.junit.Test;
@@ -44,19 +48,23 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
public BookieDecommissionTest() {
super(NUM_OF_BOOKIES, 480);
- baseConf.setOpenLedgerRereplicationGracePeriod(String.valueOf(30000));
+ baseConf.setOpenLedgerRereplicationGracePeriod(String.valueOf(1000));
setAutoRecoveryEnabled(true);
}
@FlakyTest("https://github.com/apache/bookkeeper/issues/502")
+ @Test
public void testDecommissionBookie() throws Exception {
ZkLedgerUnderreplicationManager urLedgerMgr = new ZkLedgerUnderreplicationManager(baseClientConf, zkc);
BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString());
+ List<Long> ledgerIds = new LinkedList<>();
+
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());
+ ledgerIds.add(lh.getId());
for (int j = 0; j < numOfEntries; j++) {
lh.addEntry("entry".getBytes());
}
@@ -67,6 +75,7 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
*/
for (int i = 0; i < numOfLedgers; i++) {
LedgerHandle emptylh = bkc.createLedger(3, 2, digestType, PASSWORD.getBytes());
+ ledgerIds.add(emptylh.getId());
emptylh.close();
}
@@ -88,7 +97,7 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
*/
bkAdmin.decommissionBookie(BookieImpl.getBookieId(killedBookieConf));
bkAdmin.triggerAudit();
- Thread.sleep(500);
+ Thread.sleep(5000);
Iterator<UnderreplicatedLedger> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
if (ledgersToRereplicate.hasNext()) {
while (ledgersToRereplicate.hasNext()) {
@@ -101,7 +110,7 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
killedBookieConf = killBookie(0);
bkAdmin.decommissionBookie(BookieImpl.getBookieId(killedBookieConf));
bkAdmin.triggerAudit();
- Thread.sleep(500);
+ Thread.sleep(5000);
ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
if (ledgersToRereplicate.hasNext()) {
while (ledgersToRereplicate.hasNext()) {
@@ -111,6 +120,10 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
fail("There are not supposed to be any underreplicatedledgers");
}
bkAdmin.close();
+
+ for (Long id: ledgerIds) {
+ verifyNoFragmentsOnBookie(id, BookieImpl.getBookieId(killedBookieConf));
+ }
}
@Test
@@ -130,11 +143,16 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
lh4.addEntry(j, "data".getBytes());
}
+ // avoiding autorecovery fencing the ledger
+ servers.forEach(srv -> srv.stopAutoRecovery());
+
startNewBookie();
assertEquals("Number of Available Bookies", NUM_OF_BOOKIES + 1, bkAdmin.getAvailableBookies().size());
- ServerConfiguration killedBookieConf = killBookie(0);
+ BookieId killedBookieId = getBookie(0);
+ log.warn("Killing bookie {}", killedBookieId);
+ killBookie(0);
/*
* since one of the bookie is killed, ensemble change happens when next
@@ -152,16 +170,24 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
lh1.close();
lh2.close();
+ servers.forEach(srv -> {
+ try {
+ srv.startAutoRecovery();
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ });
+
/*
* 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
+ * openLedgerRereplicationGracePeriod (by default 30 secs, 1 in the test). For more
* info. Check BOOKKEEPER-237 and BOOKKEEPER-325. But later
* ReplicationWorker will fence the ledger.
*/
- bkAdmin.decommissionBookie(BookieImpl.getBookieId(killedBookieConf));
+ bkAdmin.decommissionBookie(killedBookieId);
bkAdmin.triggerAudit();
- Thread.sleep(500);
+ Thread.sleep(5000);
Iterator<UnderreplicatedLedger> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
if (ledgersToRereplicate.hasNext()) {
while (ledgersToRereplicate.hasNext()) {
@@ -171,6 +197,73 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
fail("There are not supposed to be any underreplicatedledgers");
}
bkAdmin.close();
+
+ verifyNoFragmentsOnBookie(1L, killedBookieId);
+ verifyNoFragmentsOnBookie(2L, killedBookieId);
+ verifyNoFragmentsOnBookie(3L, killedBookieId);
+ verifyNoFragmentsOnBookie(4L, killedBookieId);
+ }
+
+ @Test
+ public void testDecommissionForEmptyLedgers() throws Exception {
+ ZkLedgerUnderreplicationManager urLedgerMgr = new ZkLedgerUnderreplicationManager(baseClientConf, zkc);
+ BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString());
+
+ LedgerHandle lh1 = bkc.createLedgerAdv(1L, numBookies, numBookies - 1, numBookies - 1,
+ digestType, PASSWORD.getBytes(), null);
+ LedgerHandle lh2 = bkc.createLedgerAdv(2L, numBookies, numBookies - 1, numBookies - 1,
+ digestType, PASSWORD.getBytes(), null);
+ LedgerHandle lh3 = bkc.createLedgerAdv(3L, numBookies, numBookies - 1, numBookies - 1,
+ digestType, PASSWORD.getBytes(), null);
+ LedgerHandle lh4 = bkc.createLedgerAdv(4L, numBookies, numBookies - 1, numBookies - 1,
+ digestType, PASSWORD.getBytes(), null);
+
+ lh1.close();
+ lh2.close();
+
+ startNewBookie();
+
+ assertEquals("Number of Available Bookies", NUM_OF_BOOKIES + 1, bkAdmin.getAvailableBookies().size());
+
+ BookieId killedBookieId = getBookie(0);
+ log.warn("Killing bookie {}", killedBookieId);
+ killBookie(0);
+ assertEquals("Number of Available Bookies", NUM_OF_BOOKIES, bkAdmin.getAvailableBookies().size());
+
+ bkAdmin.decommissionBookie(killedBookieId);
+ bkAdmin.triggerAudit();
+ Thread.sleep(5000);
+ Iterator<UnderreplicatedLedger> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
+ if (ledgersToRereplicate.hasNext()) {
+ while (ledgersToRereplicate.hasNext()) {
+ long ledgerId = ledgersToRereplicate.next().getLedgerId();
+ log.error("Ledger: {} is underreplicated which is not expected. {}",
+ ledgerId, ledgersToRereplicate.next().getReplicaList());
+ }
+ fail("There are not supposed to be any underreplicatedledgers");
+ }
+ bkAdmin.close();
+
+ verifyNoFragmentsOnBookie(1L, killedBookieId);
+ verifyNoFragmentsOnBookie(2L, killedBookieId);
+ verifyNoFragmentsOnBookie(3L, killedBookieId);
+ verifyNoFragmentsOnBookie(4L, killedBookieId);
+
+ lh3.close();
+ lh4.close();
+ }
+
+ private void verifyNoFragmentsOnBookie(long ledgerId, BookieId bookieId) throws BKException, InterruptedException {
+ LedgerHandle lh = bkc.openLedgerNoRecovery(ledgerId, digestType, PASSWORD.getBytes());
+ log.error("Ledger {} metadata: {}", ledgerId, lh.getLedgerMetadata());
+
+ lh.getLedgerMetadata().getAllEnsembles().forEach((num, bookies) -> {
+ bookies.forEach(id -> {
+ assertNotEquals(bookieId, id);
+ });
+ });
+
+ lh.close();
}
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java
index 9da4cb3fd7..d78d1599f9 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java
@@ -169,7 +169,7 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
LOG.info("unreplicated fragment: {}", r);
}
- assertEquals("Should not have any missing fragment", 0, result.size());
+ assertEquals("Empty fragment should be considered missing", 1, result.size());
}
/**
@@ -203,7 +203,7 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
LOG.info("unreplicated fragment: {}", r);
}
- assertEquals("There should be 1 fragments", 1, result.size());
+ assertEquals("Empty fragment should be considered missing", 2, result.size());
assertEquals("There should be 2 failed bookies in the fragment",
2, result.iterator().next().getBookiesIndexes().size());
}
@@ -314,8 +314,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
Set<LedgerFragment> result = getUnderReplicatedFragments(lh);
assertNotNull("Result shouldn't be null", result);
assertEquals("There should be 1 fragments.", 1, result.size());
- assertEquals("There should be 2 failed bookies in the fragment",
- 2, result.iterator().next().getBookiesIndexes().size());
+ assertEquals("There should be 3 failed bookies in the fragment",
+ 3, result.iterator().next().getBookiesIndexes().size());
}
/**
@@ -421,8 +421,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
Set<LedgerFragment> result = getUnderReplicatedFragments(lh1);
assertNotNull("Result shouldn't be null", result);
- assertEquals("There should be 0 fragment. But returned fragments are "
- + result, 0, result.size());
+ assertEquals("Empty fragment should be considered missing"
+ + result, 1, result.size());
}
/**
@@ -450,8 +450,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
Set<LedgerFragment> result = getUnderReplicatedFragments(lh1);
assertNotNull("Result shouldn't be null", result);
- assertEquals("There should be 0 fragment. But returned fragments are "
- + result, 0, result.size());
+ assertEquals("Empty fragment should be considered missing"
+ + result, 1, result.size());
lh1.close();
// kill bookie 1
@@ -469,8 +469,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
assertNotNull("Result shouldn't be null", result);
assertEquals("There should be 1 fragment. But returned fragments are "
+ result, 1, result.size());
- assertEquals("There should be 1 failed bookies in the fragment",
- 1, result.iterator().next().getBookiesIndexes().size());
+ assertEquals("There should be 2 failed bookies in the fragment",
+ 2, result.iterator().next().getBookiesIndexes().size());
lh1.close();
// kill bookie 0
@@ -488,8 +488,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
assertNotNull("Result shouldn't be null", result);
assertEquals("There should be 1 fragment. But returned fragments are "
+ result, 1, result.size());
- assertEquals("There should be 2 failed bookies in the fragment",
- 2, result.iterator().next().getBookiesIndexes().size());
+ assertEquals("There should be 3 failed bookies in the fragment",
+ 3, result.iterator().next().getBookiesIndexes().size());
lh1.close();
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/BookieAutoRecoveryTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/BookieAutoRecoveryTest.java
index a8e89dcf0c..a19be543a4 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/BookieAutoRecoveryTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/BookieAutoRecoveryTest.java
@@ -383,6 +383,7 @@ public class BookieAutoRecoveryTest extends BookKeeperClusterTestCase {
LOG.info("Killing last bookie, {}, in ensemble {}", replicaToKill,
lh.getLedgerMetadata().getAllEnsembles().get(0L));
killBookie(replicaToKill);
+ startNewBookie();
getAuditor(10, TimeUnit.SECONDS).submitAuditTask().get(); // ensure auditor runs
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java
index 1b14f17255..cb1cdaa4e8 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java
@@ -108,7 +108,7 @@ public abstract class BookKeeperClusterTestCase {
// BookKeeper related variables
protected final TmpDirs tmpDirs = new TmpDirs();
- private final List<ServerTester> servers = new LinkedList<>();
+ protected final List<ServerTester> servers = new LinkedList<>();
protected int numBookies;
protected BookKeeperTestClient bkc;
@@ -835,7 +835,7 @@ public abstract class BookKeeperClusterTestCase {
private AutoRecoveryMain autoRecovery;
- ServerTester(ServerConfiguration conf) throws Exception {
+ public ServerTester(ServerConfiguration conf) throws Exception {
this.conf = conf;
provider = new TestStatsProvider();
@@ -879,7 +879,7 @@ public abstract class BookKeeperClusterTestCase {
autoRecovery = null;
}
- ServerTester(ServerConfiguration conf, Bookie b) throws Exception {
+ public ServerTester(ServerConfiguration conf, Bookie b) throws Exception {
this.conf = conf;
provider = new TestStatsProvider();
@@ -897,20 +897,20 @@ public abstract class BookKeeperClusterTestCase {
autoRecovery = null;
}
- void startAutoRecovery() throws Exception {
+ public void startAutoRecovery() throws Exception {
LOG.debug("Starting Auditor Recovery for the bookie: {}", address);
autoRecovery = new AutoRecoveryMain(conf);
autoRecovery.start();
}
- void stopAutoRecovery() {
+ public void stopAutoRecovery() {
if (autoRecovery != null) {
LOG.debug("Shutdown Auditor Recovery for the bookie: {}", address);
autoRecovery.shutdown();
}
}
- Auditor getAuditor() {
+ public Auditor getAuditor() {
if (autoRecovery != null) {
return autoRecovery.getAuditor();
} else {
@@ -918,7 +918,7 @@ public abstract class BookKeeperClusterTestCase {
}
}
- ReplicationWorker getReplicationWorker() {
+ public ReplicationWorker getReplicationWorker() {
if (autoRecovery != null) {
return autoRecovery.getReplicationWorker();
} else {
@@ -926,7 +926,7 @@ public abstract class BookKeeperClusterTestCase {
}
}
- ServerConfiguration getConfiguration() {
+ public ServerConfiguration getConfiguration() {
return conf;
}
@@ -934,15 +934,15 @@ public abstract class BookKeeperClusterTestCase {
return server;
}
- TestStatsProvider getStatsProvider() {
+ public TestStatsProvider getStatsProvider() {
return provider;
}
- BookieSocketAddress getAddress() {
+ public BookieSocketAddress getAddress() {
return address;
}
- void shutdown() throws Exception {
+ public void shutdown() throws Exception {
server.shutdown();
if (ledgerManager != null) {