Repository: bookkeeper Updated Branches: refs/heads/master 10cab08d0 -> 91595fc04
BOOKKEEPER-911: Fix TestReplicationWorker tests in master Author: Matteo Merli <[email protected]> Reviewers: Sijie Guo <[email protected]>, Flavio Junqueira <[email protected]> Closes #29 from merlimat/bk-911 Project: http://git-wip-us.apache.org/repos/asf/bookkeeper/repo Commit: http://git-wip-us.apache.org/repos/asf/bookkeeper/commit/91595fc0 Tree: http://git-wip-us.apache.org/repos/asf/bookkeeper/tree/91595fc0 Diff: http://git-wip-us.apache.org/repos/asf/bookkeeper/diff/91595fc0 Branch: refs/heads/master Commit: 91595fc04ac6168b5a652e803e55caadae757f74 Parents: 10cab08 Author: Matteo Merli <[email protected]> Authored: Wed Apr 27 00:00:34 2016 -0700 Committer: Sijie Guo <[email protected]> Committed: Wed Apr 27 00:00:34 2016 -0700 ---------------------------------------------------------------------- .../replication/ReplicationWorker.java | 10 ++++++++- .../replication/TestReplicationWorker.java | 23 ++++++++++++-------- 2 files changed, 23 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/91595fc0/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java ---------------------------------------------------------------------- diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java index e26d09a..e686e11 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java @@ -73,6 +73,7 @@ public class ReplicationWorker implements Runnable { private final ServerConfiguration conf; private final ZooKeeper zkc; private volatile boolean workerRunning = false; + private volatile boolean isInReadOnlyMode = false; final private BookKeeperAdmin admin; private final LedgerChecker ledgerChecker; private final BookieSocketAddress targetBookie; @@ -186,9 +187,12 @@ public class ReplicationWorker implements Runnable { private void waitTillTargetBookieIsWritable() { LOG.info("Waiting for target bookie {} to be back in read/write mode", targetBookie); - while (admin.getReadOnlyBookies().contains(targetBookie)) { + while (workerRunning && admin.getReadOnlyBookies().contains(targetBookie)) { + isInReadOnlyMode = true; waitBackOffTime(); } + + isInReadOnlyMode = false; LOG.info("Target bookie {} is back in read/write mode", targetBookie); } @@ -451,6 +455,10 @@ public class ReplicationWorker implements Runnable { return workerRunning && workerThread.isAlive(); } + boolean isInReadOnlyMode() { + return isInReadOnlyMode; + } + private boolean isTargetBookieExistsInFragmentEnsemble(LedgerHandle lh, LedgerFragment ledgerFragment) { List<BookieSocketAddress> ensemble = ledgerFragment.getEnsemble(); http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/91595fc0/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestReplicationWorker.java ---------------------------------------------------------------------- diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestReplicationWorker.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestReplicationWorker.java index 8aebae4..93890fa 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestReplicationWorker.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestReplicationWorker.java @@ -506,10 +506,10 @@ public class TestReplicationWorker extends MultiLedgerManagerTestCase { } /** - * Test that if the local bookie turns out to be readonly, then no point in running RW. So RW should shutdown. + * Test that if the local bookie turns out to be read-only, then the replicator will pause but not shutdown. */ @Test(timeout = 20000) - public void testRWShutdownOnLocalBookieReadonlyTransition() throws Exception { + public void testRWOnLocalBookieReadonlyTransition() throws Exception { LedgerHandle lh = bkc.createLedger(3, 3, BookKeeper.DigestType.CRC32, TESTPASSWD); for (int i = 0; i < 10; i++) { @@ -537,21 +537,22 @@ public class TestReplicationWorker extends MultiLedgerManagerTestCase { bsConfs.get(bsConfs.size() - 1).setReadOnlyModeEnabled(true); newBk.getBookie().doTransitionToReadOnlyMode(); underReplicationManager.markLedgerUnderreplicated(lh.getId(), replicaToKill.toString()); - while (ReplicationTestUtil.isLedgerInUnderReplication(zkc, lh.getId(), basePath) && rw.isRunning()) { + while (ReplicationTestUtil.isLedgerInUnderReplication(zkc, lh.getId(), basePath) && rw.isRunning() + && !rw.isInReadOnlyMode()) { Thread.sleep(100); } assertNull(zkc.exists(String.format("%s/urL%010d", baseLockPath, lh.getId()), false)); - assertFalse("RW should shutdown if the bookie is readonly", rw.isRunning()); + assertTrue("RW should continue even if the bookie is readonly", rw.isRunning()); } finally { rw.shutdown(); } } /** - * Test that the replication worker will shutdown if it lose its zookeeper session + * Test that the replication worker will not shutdown on a simple ZK disconnection */ @Test(timeout=30000) - public void testRWZKSessionLost() throws Exception { + public void testRWZKConnectionLost() throws Exception { ZooKeeper zk = ZooKeeperClient.newBuilder() .connectString(zkUtil.getZooKeeperConnectString()) .sessionTimeoutMs(10000) @@ -567,15 +568,19 @@ public class TestReplicationWorker extends MultiLedgerManagerTestCase { Thread.sleep(1000); } assertTrue("Replication worker should be running", rw.isRunning()); - stopZKCluster(); + stopZKCluster(); + // Wait for disconnection to be picked up for (int i = 0; i < 10; i++) { - if (!rw.isRunning()) { + if (!zk.getState().isConnected()) { break; } Thread.sleep(1000); } - assertFalse("Replication worker should have shut down", rw.isRunning()); + assertFalse(zk.getState().isConnected()); + startZKCluster(); + + assertTrue("Replication worker should still be running", rw.isRunning()); } finally { zk.close(); }
