From d82293285c6b5c45194ba9f9b9005f523e5d7bfd Mon Sep 17 00:00:00 2001 From: rdhabalia Date: Wed, 6 Feb 2019 13:57:56 -0800 Subject: [PATCH 1/5] [bk-gc] Fix GC thread gets blocked --- .../main/java/org/apache/bookkeeper/util/ZkUtils.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java index 5ba4a855496..94bd745f8d1 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.util.List; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.apache.bookkeeper.conf.AbstractConfiguration; @@ -46,6 +47,7 @@ */ public class ZkUtils { private static final Logger LOG = LoggerFactory.getLogger(ZkUtils.class); + public static final int OP_TIME_OUT_SEC = 2; /** * Asynchronously create zookeeper path recursively and optimistically. @@ -240,7 +242,12 @@ public void operationComplete(int rc, List ledgers) { synchronized (ctx) { while (!ctx.done) { - ctx.wait(); + try { + ctx.wait(TimeUnit.SECONDS.toMillis(OP_TIME_OUT_SEC)); + } catch (InterruptedException e) { + ctx.rc = Code.OPERATIONTIMEOUT.intValue(); + ctx.done = true; + } } } if (Code.NONODE.intValue() == ctx.rc) { From dc51fed4012796e2f1ae1f60311f3e81af18c182 Mon Sep 17 00:00:00 2001 From: rdhabalia Date: Wed, 6 Feb 2019 15:39:13 -0800 Subject: [PATCH 2/5] add timeout in parameter --- .../java/org/apache/bookkeeper/meta/FlatLedgerManager.java | 3 ++- .../bookkeeper/meta/LegacyHierarchicalLedgerManager.java | 2 +- .../apache/bookkeeper/meta/LongHierarchicalLedgerManager.java | 2 +- .../src/main/java/org/apache/bookkeeper/util/ZkUtils.java | 4 ++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java index 7ee2e2289ea..54060d44513 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java @@ -103,7 +103,8 @@ private synchronized void preload() throws IOException { try { zkActiveLedgers = ledgerListToSet( - ZkUtils.getChildrenInSingleNode(zk, ledgerRootPath), ledgerRootPath); + ZkUtils.getChildrenInSingleNode(zk, ledgerRootPath, ZkUtils.OP_TIME_OUT_SEC), + ledgerRootPath); nextRange = new LedgerRange(zkActiveLedgers); } catch (KeeperException.NoNodeException e) { throw new IOException("Path does not exist: " + ledgerRootPath, e); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LegacyHierarchicalLedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LegacyHierarchicalLedgerManager.java index 76ecc9d68de..2b03dc75651 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LegacyHierarchicalLedgerManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LegacyHierarchicalLedgerManager.java @@ -261,7 +261,7 @@ LedgerRange getLedgerRangeByLevel(final String level1, final String level2) String nodePath = nodeBuilder.toString(); List ledgerNodes = null; try { - ledgerNodes = ZkUtils.getChildrenInSingleNode(zk, nodePath); + ledgerNodes = ZkUtils.getChildrenInSingleNode(zk, nodePath, ZkUtils.OP_TIME_OUT_SEC); } catch (KeeperException.NoNodeException e) { /* If the node doesn't exist, we must have raced with a recursive node removal, just * return an empty list. */ diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LongHierarchicalLedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LongHierarchicalLedgerManager.java index 2e69e90a5c1..c012734e3e1 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LongHierarchicalLedgerManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LongHierarchicalLedgerManager.java @@ -162,7 +162,7 @@ private class LongHierarchicalLedgerRangeIterator implements LedgerRangeIterator */ List getChildrenAt(String path) throws IOException { try { - List children = ZkUtils.getChildrenInSingleNode(zk, path); + List children = ZkUtils.getChildrenInSingleNode(zk, path, ZkUtils.OP_TIME_OUT_SEC); Collections.sort(children); return children; } catch (KeeperException.NoNodeException e) { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java index 94bd745f8d1..22e47327850 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java @@ -223,7 +223,7 @@ private static class GetChildrenCtx { * @throws InterruptedException * @throws IOException */ - public static List getChildrenInSingleNode(final ZooKeeper zk, final String node) + public static List getChildrenInSingleNode(final ZooKeeper zk, final String node, long timeOutSec) throws InterruptedException, IOException, KeeperException.NoNodeException { final GetChildrenCtx ctx = new GetChildrenCtx(); getChildrenInSingleNode(zk, node, new GenericCallback>() { @@ -243,7 +243,7 @@ public void operationComplete(int rc, List ledgers) { synchronized (ctx) { while (!ctx.done) { try { - ctx.wait(TimeUnit.SECONDS.toMillis(OP_TIME_OUT_SEC)); + ctx.wait(TimeUnit.SECONDS.toMillis(timeOutSec)); } catch (InterruptedException e) { ctx.rc = Code.OPERATIONTIMEOUT.intValue(); ctx.done = true; From 6040f2ac60e46d86ee52a1c237e0eedecff1160f Mon Sep 17 00:00:00 2001 From: rdhabalia Date: Wed, 6 Feb 2019 18:25:33 -0800 Subject: [PATCH 3/5] keep timeout configurable --- .../ScanAndCompareGarbageCollector.java | 2 +- .../bookkeeper/client/BookKeeperAdmin.java | 2 +- .../bookkeeper/conf/ServerConfiguration.java | 22 +++++++++++++++++++ .../bookkeeper/meta/CleanupLedgerManager.java | 4 ++-- .../bookkeeper/meta/FlatLedgerManager.java | 4 ++-- .../meta/HierarchicalLedgerManager.java | 6 ++--- .../apache/bookkeeper/meta/LedgerManager.java | 5 ++++- .../meta/LegacyHierarchicalLedgerManager.java | 11 +++++++--- .../meta/LongHierarchicalLedgerManager.java | 11 ++++++---- .../meta/MSLedgerManagerFactory.java | 2 +- .../http/service/ListLedgerService.java | 2 +- .../org/apache/bookkeeper/util/ZkUtils.java | 3 +-- .../bookkeeper/bookie/CompactionTest.java | 2 +- .../client/ParallelLedgerRecoveryTest.java | 4 ++-- .../apache/bookkeeper/meta/GcLedgersTest.java | 6 ++--- .../meta/LedgerManagerIteratorTest.java | 20 ++++++++--------- .../bookkeeper/meta/MockLedgerManager.java | 2 +- conf/bk_server.conf | 4 ++++ 18 files changed, 74 insertions(+), 38 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java index 24c5c97e9cc..33886905340 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java @@ -139,7 +139,7 @@ public void gc(GarbageCleaner garbageCleaner) { } // Iterate over all the ledger on the metadata store - LedgerRangeIterator ledgerRangeIterator = ledgerManager.getLedgerRanges(); + LedgerRangeIterator ledgerRangeIterator = ledgerManager.getLedgerRanges(this.conf.getZkOperationTimeOut()); Set ledgersInMetadata = null; long start; long end = -1; 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 37b59d189f5..9303ffdff8c 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 @@ -1320,7 +1320,7 @@ private static boolean validateDirectoriesAreEmpty(File[] dirs, String typeOfDir */ public Iterable listLedgers() throws IOException { - final LedgerRangeIterator iterator = bkc.getLedgerManager().getLedgerRanges(); + final LedgerRangeIterator iterator = bkc.getLedgerManager().getLedgerRanges(0); return new Iterable() { public Iterator iterator() { return new Iterator() { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java index f4972f0a781..868d16e0907 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java @@ -104,6 +104,7 @@ public class ServerConfiguration extends AbstractConfiguration processor, } @Override - public LedgerRangeIterator getLedgerRanges() { + public LedgerRangeIterator getLedgerRanges(long zkOpTimeOutSec) { return new LedgerRangeIterator() { // single iterator, can visit only one time boolean nextCalled = false; @@ -103,7 +103,7 @@ private synchronized void preload() throws IOException { try { zkActiveLedgers = ledgerListToSet( - ZkUtils.getChildrenInSingleNode(zk, ledgerRootPath, ZkUtils.OP_TIME_OUT_SEC), + ZkUtils.getChildrenInSingleNode(zk, ledgerRootPath, zkOpTimeOutSec), ledgerRootPath); nextRange = new LedgerRange(zkActiveLedgers); } catch (KeeperException.NoNodeException e) { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java index 946ed2a5d32..079fcfacd25 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java @@ -87,9 +87,9 @@ protected long getLedgerId(String ledgerPath) throws IOException { } @Override - public LedgerRangeIterator getLedgerRanges() { - LedgerRangeIterator legacyLedgerRangeIterator = legacyLM.getLedgerRanges(); - LedgerRangeIterator longLedgerRangeIterator = longLM.getLedgerRanges(); + public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) { + LedgerRangeIterator legacyLedgerRangeIterator = legacyLM.getLedgerRanges(zkOpTimeoutSec); + LedgerRangeIterator longLedgerRangeIterator = longLM.getLedgerRanges(zkOpTimeoutSec); return new HierarchicalLedgerRangeIterator(legacyLedgerRangeIterator, longLedgerRangeIterator); } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java index cc7630b10b8..039ff7ccbde 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java @@ -149,9 +149,12 @@ void asyncProcessLedgers(Processor processor, AsyncCallback.VoidCallback f /** * Loop to scan a range of metadata from metadata storage. * + * @param zkOpTimeOutSec + * Iterator considers timeout while fetching ledger-range from + * zk. * @return will return a iterator of the Ranges */ - LedgerRangeIterator getLedgerRanges(); + LedgerRangeIterator getLedgerRanges(long zkOpTimeOutSec); /** * Used to represent the Ledgers range returned from the diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LegacyHierarchicalLedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LegacyHierarchicalLedgerManager.java index 2b03dc75651..1a63407e7ad 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LegacyHierarchicalLedgerManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LegacyHierarchicalLedgerManager.java @@ -153,8 +153,8 @@ protected String getLedgerParentNodeRegex() { } @Override - public LedgerRangeIterator getLedgerRanges() { - return new LegacyHierarchicalLedgerRangeIterator(); + public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) { + return new LegacyHierarchicalLedgerRangeIterator(zkOpTimeoutSec); } /** @@ -166,6 +166,11 @@ private class LegacyHierarchicalLedgerRangeIterator implements LedgerRangeIterat private String curL1Nodes = ""; private boolean iteratorDone = false; private LedgerRange nextRange = null; + private final long zkOpTimeoutSec; + + public LegacyHierarchicalLedgerRangeIterator(long zkOpTimeoutSec) { + this.zkOpTimeoutSec = zkOpTimeoutSec; + } /** * Iterate next level1 znode. @@ -261,7 +266,7 @@ LedgerRange getLedgerRangeByLevel(final String level1, final String level2) String nodePath = nodeBuilder.toString(); List ledgerNodes = null; try { - ledgerNodes = ZkUtils.getChildrenInSingleNode(zk, nodePath, ZkUtils.OP_TIME_OUT_SEC); + ledgerNodes = ZkUtils.getChildrenInSingleNode(zk, nodePath, zkOpTimeoutSec); } catch (KeeperException.NoNodeException e) { /* If the node doesn't exist, we must have raced with a recursive node removal, just * return an empty list. */ diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LongHierarchicalLedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LongHierarchicalLedgerManager.java index c012734e3e1..95f8e48bde8 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LongHierarchicalLedgerManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LongHierarchicalLedgerManager.java @@ -139,8 +139,8 @@ public void process(String lNode, VoidCallback cb) { } @Override - public LedgerRangeIterator getLedgerRanges() { - return new LongHierarchicalLedgerRangeIterator(); + public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) { + return new LongHierarchicalLedgerRangeIterator(zkOpTimeoutSec); } @@ -149,6 +149,7 @@ public LedgerRangeIterator getLedgerRanges() { */ private class LongHierarchicalLedgerRangeIterator implements LedgerRangeIterator { LedgerRangeIterator rootIterator; + final long zkOpTimeoutSec; /** * Returns all children with path as a parent. If path is non-existent, @@ -162,7 +163,7 @@ private class LongHierarchicalLedgerRangeIterator implements LedgerRangeIterator */ List getChildrenAt(String path) throws IOException { try { - List children = ZkUtils.getChildrenInSingleNode(zk, path, ZkUtils.OP_TIME_OUT_SEC); + List children = ZkUtils.getChildrenInSingleNode(zk, path, zkOpTimeoutSec); Collections.sort(children); return children; } catch (KeeperException.NoNodeException e) { @@ -284,7 +285,9 @@ public LedgerRange next() throws IOException { } } - private LongHierarchicalLedgerRangeIterator() {} + private LongHierarchicalLedgerRangeIterator(long zkOpTimeoutSec) { + this.zkOpTimeoutSec = zkOpTimeoutSec; + } private void bootstrap() throws IOException { if (rootIterator == null) { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java index 0834147a8c5..266db3a3fbc 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java @@ -641,7 +641,7 @@ public LedgerRange next() throws IOException { } @Override - public LedgerRangeIterator getLedgerRanges() { + public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) { return new MSLedgerRangeIterator(); } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/ListLedgerService.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/ListLedgerService.java index f553b6431b9..1683fd4a887 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/ListLedgerService.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/ListLedgerService.java @@ -90,7 +90,7 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception { LedgerManagerFactory mFactory = bookieServer.getBookie().getLedgerManagerFactory(); LedgerManager manager = mFactory.newLedgerManager(); - LedgerManager.LedgerRangeIterator iter = manager.getLedgerRanges(); + LedgerManager.LedgerRangeIterator iter = manager.getLedgerRanges(0); // output LinkedHashMap output = Maps.newLinkedHashMap(); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java index 22e47327850..23d0b421c3e 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java @@ -47,7 +47,6 @@ */ public class ZkUtils { private static final Logger LOG = LoggerFactory.getLogger(ZkUtils.class); - public static final int OP_TIME_OUT_SEC = 2; /** * Asynchronously create zookeeper path recursively and optimistically. @@ -243,7 +242,7 @@ public void operationComplete(int rc, List ledgers) { synchronized (ctx) { while (!ctx.done) { try { - ctx.wait(TimeUnit.SECONDS.toMillis(timeOutSec)); + ctx.wait(timeOutSec > 0 ? TimeUnit.SECONDS.toMillis(timeOutSec) : 0); } catch (InterruptedException e) { ctx.rc = Code.OPERATIONTIMEOUT.intValue(); ctx.done = true; diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java index 556556e348d..3d7d2c4aa1a 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java @@ -970,7 +970,7 @@ void unsupported() { } @Override - public LedgerRangeIterator getLedgerRanges() { + public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) { final AtomicBoolean hasnext = new AtomicBoolean(true); return new LedgerManager.LedgerRangeIterator() { @Override diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ParallelLedgerRecoveryTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ParallelLedgerRecoveryTest.java index ab966fb4ae2..e6c3c222928 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ParallelLedgerRecoveryTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ParallelLedgerRecoveryTest.java @@ -112,8 +112,8 @@ public CompletableFuture> readLedgerMetadata(long ledg } @Override - public LedgerRangeIterator getLedgerRanges() { - return lm.getLedgerRanges(); + public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) { + return lm.getLedgerRanges(zkOpTimeoutSec); } @Override diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java index 933c11763f0..abc66144a72 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java @@ -316,7 +316,7 @@ public void clean(long ledgerId) { }; SortedSet scannedLedgers = new TreeSet(); - LedgerRangeIterator iterator = getLedgerManager().getLedgerRanges(); + LedgerRangeIterator iterator = getLedgerManager().getLedgerRanges(0); while (iterator.hasNext()) { LedgerRange ledgerRange = iterator.next(); scannedLedgers.addAll(ledgerRange.getLedgers()); @@ -422,7 +422,7 @@ public void testGcLedgersIfLedgerManagerIteratorFails() throws Exception { LedgerManager mockLedgerManager = new CleanupLedgerManager(getLedgerManager()) { @Override - public LedgerRangeIterator getLedgerRanges() { + public LedgerRangeIterator getLedgerRanges(long zkOpTimeout) { return new LedgerRangeIterator() { @Override public LedgerRange next() throws IOException { @@ -552,7 +552,7 @@ public void clean(long ledgerId) { public void validateLedgerRangeIterator(SortedSet createdLedgers) throws IOException { SortedSet scannedLedgers = new TreeSet(); - LedgerRangeIterator iterator = getLedgerManager().getLedgerRanges(); + LedgerRangeIterator iterator = getLedgerManager().getLedgerRanges(0); while (iterator.hasNext()) { LedgerRange ledgerRange = iterator.next(); scannedLedgers.addAll(ledgerRange.getLedgers()); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerIteratorTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerIteratorTest.java index bdb29029766..1593e4bab98 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerIteratorTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerIteratorTest.java @@ -134,7 +134,7 @@ static Set getLedgerIdsByUsingAsyncProcessLedgers(LedgerManager lm) throws @Test public void testIterateNoLedgers() throws Exception { LedgerManager lm = getLedgerManager(); - LedgerRangeIterator lri = lm.getLedgerRanges(); + LedgerRangeIterator lri = lm.getLedgerRanges(0); assertNotNull(lri); if (lri.hasNext()) { lri.next(); @@ -150,7 +150,7 @@ public void testSingleLedger() throws Throwable { long id = 2020202; createLedger(lm, id); - LedgerRangeIterator lri = lm.getLedgerRanges(); + LedgerRangeIterator lri = lm.getLedgerRanges(0); assertNotNull(lri); Set lids = ledgerRangeToSet(lri); assertEquals(lids.size(), 1); @@ -169,7 +169,7 @@ public void testTwoLedgers() throws Throwable { createLedger(lm, id); } - LedgerRangeIterator lri = lm.getLedgerRanges(); + LedgerRangeIterator lri = lm.getLedgerRanges(0); assertNotNull(lri); Set returnedIds = ledgerRangeToSet(lri); assertEquals(ids, returnedIds); @@ -188,7 +188,7 @@ public void testSeveralContiguousLedgers() throws Throwable { ids.add(i); } - LedgerRangeIterator lri = lm.getLedgerRanges(); + LedgerRangeIterator lri = lm.getLedgerRanges(0); assertNotNull(lri); Set returnedIds = ledgerRangeToSet(lri); assertEquals(ids, returnedIds); @@ -232,7 +232,7 @@ public void testRemovalOfNodeJustTraversed() throws Throwable { } Set found = new TreeSet<>(); - LedgerRangeIterator lri = lm.getLedgerRanges(); + LedgerRangeIterator lri = lm.getLedgerRanges(0); while (lri.hasNext()) { LedgerManager.LedgerRange lr = lri.next(); found.addAll(lr.getLedgers()); @@ -279,7 +279,7 @@ public void validateEmptyL4PathSkipped() throws Throwable { path, "data".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); } - LedgerRangeIterator lri = lm.getLedgerRanges(); + LedgerRangeIterator lri = lm.getLedgerRanges(0); assertNotNull(lri); Set returnedIds = ledgerRangeToSet(lri); assertEquals(ids, returnedIds); @@ -287,7 +287,7 @@ public void validateEmptyL4PathSkipped() throws Throwable { Set ledgersReadAsync = getLedgerIdsByUsingAsyncProcessLedgers(lm); assertEquals("Comparing LedgersIds read asynchronously", ids, ledgersReadAsync); - lri = lm.getLedgerRanges(); + lri = lm.getLedgerRanges(0); int emptyRanges = 0; while (lri.hasNext()) { if (lri.next().getLedgers().isEmpty()) { @@ -329,7 +329,7 @@ public void testWithSeveralIncompletePaths() throws Throwable { path, "data".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); } - LedgerRangeIterator lri = lm.getLedgerRanges(); + LedgerRangeIterator lri = lm.getLedgerRanges(0); assertNotNull(lri); Set returnedIds = ledgerRangeToSet(lri); assertEquals(ids, returnedIds); @@ -394,7 +394,7 @@ public void checkConcurrentModifications() throws Throwable { latch.await(); while (MathUtils.elapsedNanos(start) < runtime) { - LedgerRangeIterator lri = checkerLM.getLedgerRanges(); + LedgerRangeIterator lri = checkerLM.getLedgerRanges(0); Set returnedIds = ledgerRangeToSet(lri); for (long id: mustExist) { assertTrue(returnedIds.contains(id)); @@ -505,7 +505,7 @@ public void testLedgerManagerFormat() throws Throwable { public void hierarchicalLedgerManagerAsyncProcessLedgersTest() throws Throwable { Assume.assumeTrue(baseConf.getLedgerManagerFactoryClass().equals(HierarchicalLedgerManagerFactory.class)); LedgerManager lm = getLedgerManager(); - LedgerRangeIterator lri = lm.getLedgerRanges(); + LedgerRangeIterator lri = lm.getLedgerRanges(0); Set ledgerIds = new TreeSet<>(Arrays.asList(1234L, 123456789123456789L)); for (Long ledgerId : ledgerIds) { diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/MockLedgerManager.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/MockLedgerManager.java index f5cbe3a7e65..0f818b66c9e 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/MockLedgerManager.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/MockLedgerManager.java @@ -188,7 +188,7 @@ public void asyncProcessLedgers(Processor processor, AsyncCallback.VoidCal } @Override - public LedgerRangeIterator getLedgerRanges() { + public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) { return null; } diff --git a/conf/bk_server.conf b/conf/bk_server.conf index 41798a6df4b..e804dcc532e 100755 --- a/conf/bk_server.conf +++ b/conf/bk_server.conf @@ -534,6 +534,10 @@ ledgerDirectories=/tmp/bk-data # True if the bookie should double check readMetadata prior to gc # verifyMetadataOnGC=false +# Get zk-operation timeout in seconds used by GC while performing zk +# operation (set 0 to ignore timeout. default = 0). +# zkOperationTimeout=0 + ############################################################################# ## Disk utilization ############################################################################# From ddc482819578b38a803edf9d42d8477b0c830033 Mon Sep 17 00:00:00 2001 From: rdhabalia Date: Thu, 7 Feb 2019 14:51:54 -0800 Subject: [PATCH 4/5] derive zkoptimeout from zktimeout --- .../ScanAndCompareGarbageCollector.java | 3 ++- .../bookkeeper/conf/ServerConfiguration.java | 22 ------------------- conf/bk_server.conf | 4 ---- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java index 33886905340..b1e77229c13 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java @@ -139,7 +139,8 @@ public void gc(GarbageCleaner garbageCleaner) { } // Iterate over all the ledger on the metadata store - LedgerRangeIterator ledgerRangeIterator = ledgerManager.getLedgerRanges(this.conf.getZkOperationTimeOut()); + long zkOpTimeout = this.conf.getZkTimeout() * 2; + LedgerRangeIterator ledgerRangeIterator = ledgerManager.getLedgerRanges(zkOpTimeout); Set ledgersInMetadata = null; long start; long end = -1; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java index 868d16e0907..f4972f0a781 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java @@ -104,7 +104,6 @@ public class ServerConfiguration extends AbstractConfiguration Date: Fri, 8 Feb 2019 14:55:12 -0800 Subject: [PATCH 5/5] fix EtcdLedgerManager --- .../org/apache/bookkeeper/metadata/etcd/EtcdLedgerManager.java | 2 +- .../apache/bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/metadata-drivers/etcd/src/main/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManager.java b/metadata-drivers/etcd/src/main/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManager.java index 4988a810fca..a40a4bc383b 100644 --- a/metadata-drivers/etcd/src/main/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManager.java +++ b/metadata-drivers/etcd/src/main/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManager.java @@ -420,7 +420,7 @@ private void processLedgers(KeyStream ks, } @Override - public LedgerRangeIterator getLedgerRanges() { + public LedgerRangeIterator getLedgerRanges(long opTimeOutSec) { KeyStream ks = new KeyStream<>( kvClient, ByteSequence.fromString(EtcdUtils.getLedgerKey(scope, 0L)), diff --git a/metadata-drivers/etcd/src/test/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java b/metadata-drivers/etcd/src/test/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java index 984224c6b8b..992d24f09be 100644 --- a/metadata-drivers/etcd/src/test/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java +++ b/metadata-drivers/etcd/src/test/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java @@ -225,7 +225,7 @@ public void testLedgerRangeIterator() throws Exception { createNumLedgers(numLedgers); long nextLedgerId = 0L; - LedgerRangeIterator iter = lm.getLedgerRanges(); + LedgerRangeIterator iter = lm.getLedgerRanges(0); while (iter.hasNext()) { LedgerRange lr = iter.next(); for (Long lid : lr.getLedgers()) {