Skip to content

[bk-gc] avoid blocking call in gc-thread #1940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
eolivelli merged 2 commits into from
May 15, 2019
Merged

[bk-gc] avoid blocking call in gc-thread #1940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9093bf
[bk-gc] avoid blocking call while gc checking deleted ledger znode
rdhabalia Feb 6, 2019
392b870
check disable by verifying zkOpTimeoutMs=0
rdhabalia May 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions ...per-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
import java.util.TreeSet;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

import org.apache.bookkeeper.client.BKException;
import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.meta.LedgerManager;
Expand Down Expand Up @@ -139,8 +142,9 @@ public void gc(GarbageCleaner garbageCleaner) {
}

// Iterate over all the ledger on the metadata store
long zkOpTimeout = this.conf.getZkTimeout() * 2;
LedgerRangeIterator ledgerRangeIterator = ledgerManager.getLedgerRanges(zkOpTimeout);
long zkOpTimeoutMs = this.conf.getZkTimeout() * 2;
LedgerRangeIterator ledgerRangeIterator = ledgerManager
.getLedgerRanges(zkOpTimeoutMs);
Set<Long> ledgersInMetadata = null;
long start;
long end = -1;
Expand All @@ -167,13 +171,20 @@ public void gc(GarbageCleaner garbageCleaner) {
if (verifyMetadataOnGc) {
int rc = BKException.Code.OK;
try {
result(ledgerManager.readLedgerMetadata(bkLid));
} catch (BKException e) {
rc = e.getCode();
result(ledgerManager.readLedgerMetadata(bkLid), zkOpTimeoutMs, TimeUnit.MILLISECONDS);
} catch (BKException | TimeoutException e) {
if (e instanceof BKException) {
rc = ((BKException) e).getCode();
} else {
LOG.warn("Time-out while fetching metadata for Ledger {} : {}.", bkLid,
e.getMessage());

continue;
}
}
if (rc != BKException.Code.NoSuchLedgerExistsOnMetadataServerException) {
LOG.warn("Ledger {} Missing in metadata list, but ledgerManager returned rc: {}.",
bkLid, rc);
bkLid, rc);
continue;
}
}
Expand Down
4 changes: 2 additions & 2 deletions bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,13 @@ public void processResult(int rc, String path, Object ctx) {
}

@Override
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) {
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutMs) {
closeLock.readLock().lock();
try {
if (closed) {
return new ClosedLedgerRangeIterator();
}
return underlying.getLedgerRanges(zkOpTimeoutSec);
return underlying.getLedgerRanges(zkOpTimeoutMs);
} finally {
closeLock.readLock().unlock();
}
Expand Down
4 changes: 2 additions & 2 deletions bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void asyncProcessLedgers(final Processor<Long> processor,
}

@Override
public LedgerRangeIterator getLedgerRanges(long zkOpTimeOutSec) {
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutMs) {
return new LedgerRangeIterator() {
// single iterator, can visit only one time
boolean nextCalled = false;
Expand All @@ -103,7 +103,7 @@ private synchronized void preload() throws IOException {

try {
zkActiveLedgers = ledgerListToSet(
ZkUtils.getChildrenInSingleNode(zk, ledgerRootPath, zkOpTimeOutSec),
ZkUtils.getChildrenInSingleNode(zk, ledgerRootPath, zkOpTimeoutMs),
ledgerRootPath);
nextRange = new LedgerRange(zkActiveLedgers);
} catch (KeeperException.NoNodeException e) {
Expand Down
6 changes: 3 additions & 3 deletions bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ protected long getLedgerId(String ledgerPath) throws IOException {
}

@Override
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) {
LedgerRangeIterator legacyLedgerRangeIterator = legacyLM.getLedgerRanges(zkOpTimeoutSec);
LedgerRangeIterator longLedgerRangeIterator = longLM.getLedgerRanges(zkOpTimeoutSec);
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutMs) {
LedgerRangeIterator legacyLedgerRangeIterator = legacyLM.getLedgerRanges(zkOpTimeoutMs);
LedgerRangeIterator longLedgerRangeIterator = longLM.getLedgerRanges(zkOpTimeoutMs);
return new HierarchicalLedgerRangeIterator(legacyLedgerRangeIterator, longLedgerRangeIterator);
}

Expand Down
4 changes: 2 additions & 2 deletions bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,12 @@ void asyncProcessLedgers(Processor<Long> processor, AsyncCallback.VoidCallback f
/**
* Loop to scan a range of metadata from metadata storage.
*
* @param zkOpTimeOutSec
* @param zkOpTimeOutMs
* Iterator considers timeout while fetching ledger-range from
* zk.
* @return will return a iterator of the Ranges
*/
LedgerRangeIterator getLedgerRanges(long zkOpTimeOutSec);
LedgerRangeIterator getLedgerRanges(long zkOpTimeOutMs);

/**
* Used to represent the Ledgers range returned from the
Expand Down
12 changes: 6 additions & 6 deletions ...eper-server/src/main/java/org/apache/bookkeeper/meta/LegacyHierarchicalLedgerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ protected String getLedgerParentNodeRegex() {
}

@Override
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) {
return new LegacyHierarchicalLedgerRangeIterator(zkOpTimeoutSec);
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutMs) {
return new LegacyHierarchicalLedgerRangeIterator(zkOpTimeoutMs);
}

/**
Expand All @@ -166,10 +166,10 @@ private class LegacyHierarchicalLedgerRangeIterator implements LedgerRangeIterat
private String curL1Nodes = "";
private boolean iteratorDone = false;
private LedgerRange nextRange = null;
private final long zkOpTimeoutSec;
private final long zkOpTimeoutMs;

public LegacyHierarchicalLedgerRangeIterator(long zkOpTimeoutSec) {
this.zkOpTimeoutSec = zkOpTimeoutSec;
public LegacyHierarchicalLedgerRangeIterator(long zkOpTimeoutMs) {
this.zkOpTimeoutMs = zkOpTimeoutMs;
}

/**
Expand Down Expand Up @@ -266,7 +266,7 @@ LedgerRange getLedgerRangeByLevel(final String level1, final String level2)
String nodePath = nodeBuilder.toString();
List<String> ledgerNodes = null;
try {
ledgerNodes = ZkUtils.getChildrenInSingleNode(zk, nodePath, zkOpTimeoutSec);
ledgerNodes = ZkUtils.getChildrenInSingleNode(zk, nodePath, zkOpTimeoutMs);
} catch (KeeperException.NoNodeException e) {
/* If the node doesn't exist, we must have raced with a recursive node removal, just
* return an empty list. */
Expand Down
12 changes: 6 additions & 6 deletions ...keeper-server/src/main/java/org/apache/bookkeeper/meta/LongHierarchicalLedgerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ public void process(String lNode, VoidCallback cb) {
}

@Override
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) {
return new LongHierarchicalLedgerRangeIterator(zkOpTimeoutSec);
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutMs) {
return new LongHierarchicalLedgerRangeIterator(zkOpTimeoutMs);
}


Expand All @@ -149,7 +149,7 @@ public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) {
*/
private class LongHierarchicalLedgerRangeIterator implements LedgerRangeIterator {
LedgerRangeIterator rootIterator;
final long zkOpTimeoutSec;
final long zkOpTimeoutMs;

/**
* Returns all children with path as a parent. If path is non-existent,
Expand All @@ -163,7 +163,7 @@ private class LongHierarchicalLedgerRangeIterator implements LedgerRangeIterator
*/
List<String> getChildrenAt(String path) throws IOException {
try {
List<String> children = ZkUtils.getChildrenInSingleNode(zk, path, zkOpTimeoutSec);
List<String> children = ZkUtils.getChildrenInSingleNode(zk, path, zkOpTimeoutMs);
Collections.sort(children);
return children;
} catch (KeeperException.NoNodeException e) {
Expand Down Expand Up @@ -285,8 +285,8 @@ public LedgerRange next() throws IOException {
}
}

private LongHierarchicalLedgerRangeIterator(long zkOpTimeoutSec) {
this.zkOpTimeoutSec = zkOpTimeoutSec;
private LongHierarchicalLedgerRangeIterator(long zkOpTimeoutMs) {
this.zkOpTimeoutMs = zkOpTimeoutMs;
}

private void bootstrap() throws IOException {
Expand Down
2 changes: 1 addition & 1 deletion bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ public LedgerRange next() throws IOException {
}

@Override
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) {
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutMs) {
return new MSLedgerRangeIterator();
}

Expand Down
12 changes: 9 additions & 3 deletions bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
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;
Expand Down Expand Up @@ -222,7 +221,7 @@ private static class GetChildrenCtx {
* @throws InterruptedException
* @throws IOException
*/
public static List<String> getChildrenInSingleNode(final ZooKeeper zk, final String node, long timeOutSec)
public static List<String> getChildrenInSingleNode(final ZooKeeper zk, final String node, long zkOpTimeoutMs)
throws InterruptedException, IOException, KeeperException.NoNodeException {
final GetChildrenCtx ctx = new GetChildrenCtx();
getChildrenInSingleNode(zk, node, new GenericCallback<List<String>>() {
Expand All @@ -240,13 +239,20 @@ public void operationComplete(int rc, List<String> ledgers) {
});

synchronized (ctx) {
long startTime = System.currentTimeMillis();
while (!ctx.done) {
try {
ctx.wait(timeOutSec > 0 ? TimeUnit.SECONDS.toMillis(timeOutSec) : 0);
ctx.wait(zkOpTimeoutMs > 0 ? zkOpTimeoutMs : 0);
} catch (InterruptedException e) {
ctx.rc = Code.OPERATIONTIMEOUT.intValue();
ctx.done = true;
}
// timeout the process if get-children response not received
// zkOpTimeoutMs.
if (zkOpTimeoutMs > 0 && (System.currentTimeMillis() - startTime) >= zkOpTimeoutMs) {
ctx.rc = Code.OPERATIONTIMEOUT.intValue();
ctx.done = true;
}
}
}
if (Code.NONODE.intValue() == ctx.rc) {
Expand Down
2 changes: 1 addition & 1 deletion bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ void unsupported() {
}

@Override
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) {
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutMs) {
final AtomicBoolean hasnext = new AtomicBoolean(true);
return new LedgerManager.LedgerRangeIterator() {
@Override
Expand Down
4 changes: 2 additions & 2 deletions bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ParallelLedgerRecoveryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ public CompletableFuture<Versioned<LedgerMetadata>> readLedgerMetadata(long ledg
}

@Override
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) {
return lm.getLedgerRanges(zkOpTimeoutSec);
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutMs) {
return lm.getLedgerRanges(zkOpTimeoutMs);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/MockLedgerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public void asyncProcessLedgers(Processor<Long> processor, AsyncCallback.VoidCal
}

@Override
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutSec) {
public LedgerRangeIterator getLedgerRanges(long zkOpTimeoutMs) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion ...ata-drivers/etcd/src/main/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ private void processLedgers(KeyStream<Long> ks,
}

@Override
public LedgerRangeIterator getLedgerRanges(long opTimeOutSec) {
public LedgerRangeIterator getLedgerRanges(long opTimeOutMs) {
KeyStream<Long> ks = new KeyStream<>(
kvClient,
ByteSequence.fromString(EtcdUtils.getLedgerKey(scope, 0L)),
Expand Down