Repository: bookkeeper Updated Branches: refs/heads/master 492997b1e -> 2b88b11fa
BOOKKEEPER-804: Client program is not terminated when using openLedgerNoRecovery (ivank via sijie) Project: http://git-wip-us.apache.org/repos/asf/bookkeeper/repo Commit: http://git-wip-us.apache.org/repos/asf/bookkeeper/commit/2b88b11f Tree: http://git-wip-us.apache.org/repos/asf/bookkeeper/tree/2b88b11f Diff: http://git-wip-us.apache.org/repos/asf/bookkeeper/diff/2b88b11f Branch: refs/heads/master Commit: 2b88b11fa7597254ccbf828f16efc665e852788d Parents: 492997b Author: Sijie Guo <[email protected]> Authored: Fri Dec 5 22:38:44 2014 -0800 Committer: Sijie Guo <[email protected]> Committed: Fri Dec 5 22:38:44 2014 -0800 ---------------------------------------------------------------------- CHANGES.txt | 2 + .../meta/AbstractZkLedgerManager.java | 1 - .../bookkeeper/meta/CleanupLedgerManager.java | 1 + .../bookkeeper/client/BookKeeperCloseTest.java | 58 ++++++++++++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/2b88b11f/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 203f2d2..a5ad341 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -22,6 +22,8 @@ Trunk (unreleased changes) BOOKKEEPER-809: Wrong metric on LedgerDeleteOp and LedgerOpenOp (Charles Xie via sijie) + BOOKKEEPER-804: Client program is not terminated when using openLedgerNoRecovery (ivank via sijie) + IMPROVEMENTS: BOOKKEEPER-800: Expose whether a ledger is closed or not (ivank) http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/2b88b11f/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java ---------------------------------------------------------------------- diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java index 0fc8afe..f3f680d 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java @@ -134,7 +134,6 @@ abstract class AbstractZkLedgerManager implements LedgerManager, Watcher { this.conf = conf; this.zk = zk; this.ledgerRootPath = conf.getZkLedgersRootPath(); - this.scheduler = Executors.newSingleThreadScheduledExecutor(); ThreadFactoryBuilder tfb = new ThreadFactoryBuilder().setNameFormat( "ZkLedgerManagerScheduler-%d"); this.scheduler = Executors http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/2b88b11f/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java ---------------------------------------------------------------------- diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java index a873112..a7fbcf5 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java @@ -222,5 +222,6 @@ public class CleanupLedgerManager implements LedgerManager { callback.operationComplete(BKException.Code.ClientClosedException, null); } } + underlying.close(); } } http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/2b88b11f/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperCloseTest.java ---------------------------------------------------------------------- diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperCloseTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperCloseTest.java index 89790f6..13ccfe5 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperCloseTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperCloseTest.java @@ -37,6 +37,8 @@ import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.util.concurrent.SettableFuture; + import java.io.IOException; import java.nio.ByteBuffer; import java.util.Enumeration; @@ -522,6 +524,62 @@ public class BookKeeperCloseTest extends BookKeeperClusterTestCase { } } + /** + * Test that the bookkeeper client doesn't leave any threads hanging around. + * See {@link https://issues.apache.org/jira/browse/BOOKKEEPER-804} + */ + @Test(timeout = 60000) + public void testBookKeeperCloseThreads() throws Exception { + ThreadGroup group = new ThreadGroup("test-group"); + final SettableFuture<Void> future = SettableFuture.<Void>create(); + + Thread t = new Thread(group, "TestThread") { + @Override + public void run() { + try { + BookKeeper bk = new BookKeeper(baseClientConf); + // 9 is a ledger id of an existing ledger + LedgerHandle lh = bk.createLedger(BookKeeper.DigestType.CRC32, "passwd".getBytes()); + lh.addEntry("foobar".getBytes()); + lh.close(); + long id = lh.getId(); + // 9 is a ledger id of an existing ledger + lh = bk.openLedgerNoRecovery(id, BookKeeper.DigestType.CRC32, "passwd".getBytes()); + Enumeration<LedgerEntry> entries = lh.readEntries(0, 0); + + lh.close(); + bk.close(); + future.set(null); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + future.setException(ie); + } catch (Exception e) { + future.setException(e); + } + } + }; + t.start(); + + future.get(); + t.join(); + + // check in a loop for 10 seconds + // because sometimes it takes a while to threads to go away + for (int i = 0; i < 10; i++) { + if (group.activeCount() > 0) { + Thread[] threads = new Thread[group.activeCount()]; + group.enumerate(threads); + for (Thread leftover : threads) { + LOG.error("Leftover thread after {} secs: {}", i, leftover); + } + Thread.sleep(1000); + } else { + break; + } + } + assertEquals("Should be no threads left in group", 0, group.activeCount()); + } + private LedgerHandle createLedgerWithEntries(BookKeeper bk, int numOfEntries) throws Exception { LedgerHandle lh = bk
