Author: ivank Date: Mon Sep 9 10:29:09 2013 New Revision: 1521028 URL: http://svn.apache.org/r1521028 Log: BOOKKEEPER-679: Bookie should exit with non-zero if NIOServer crashes with Error (ivank) Modified: zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieShutdownTest.java Modified: zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt?rev=1521028&r1=1521027&r2=1521028&view=diff ============================================================================== --- zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt (original) +++ zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt Mon Sep 9 10:29:09 2013 @@ -72,6 +72,8 @@ Release 4.2.2 - Unreleased BOOKKEEPER-664: Compaction increases latency on journal writes (ivank & sijie via ivank) + BOOKKEEPER-679: Bookie should exit with non-zero if NIOServer crashes with Error (ivank) + hedwig-server: BOOKKEEPER-579: TestSubAfterCloseSub was put in a wrong package (sijie via ivank) Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java?rev=1521028&r1=1521027&r2=1521028&view=diff ============================================================================== --- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java (original) +++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java Mon Sep 9 10:29:09 2013 @@ -70,8 +70,6 @@ public class BookieServer implements NIO DeathWatcher deathWatcher; static Logger LOG = LoggerFactory.getLogger(BookieServer.class); - int exitCode = ExitCode.OK; - // operation stats final BKStats bkStats = BKStats.getInstance(); final boolean isStatsEnabled; @@ -102,7 +100,6 @@ public class BookieServer implements NIO this.bookie.start(); // fail fast, when bookie startup is not successful if (!this.bookie.isRunning()) { - exitCode = bookie.getExitCode(); return; } if (isAutoRecoveryDaemonEnabled && this.autoRecoveryMain != null) { @@ -153,7 +150,8 @@ public class BookieServer implements NIO return; } nioServerFactory.shutdown(); - exitCode = bookie.shutdown(); + bookie.shutdown(); + if (isAutoRecoveryDaemonEnabled && this.autoRecoveryMain != null) { this.autoRecoveryMain.shutdown(); } @@ -224,6 +222,12 @@ public class BookieServer implements NIO } public int getExitCode() { + int exitCode = bookie.getExitCode(); + if (exitCode == ExitCode.OK) { + if (nioServerFactory.hasCrashed()) { + return ExitCode.SERVER_EXCEPTION; + } + } return exitCode; } Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java?rev=1521028&r1=1521027&r2=1521028&view=diff ============================================================================== --- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java (original) +++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java Mon Sep 9 10:29:09 2013 @@ -33,6 +33,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.Set; import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.bookkeeper.bookie.Bookie; import org.apache.bookkeeper.conf.ServerConfiguration; @@ -77,6 +78,7 @@ public class NIOServerFactory extends Th ServerConfiguration conf; + private AtomicBoolean crashed = new AtomicBoolean(false); private Object suspensionLock = new Object(); private boolean suspended = false; @@ -111,6 +113,10 @@ public class NIOServerFactory extends Th return !ss.socket().isClosed() && isAlive(); } + boolean hasCrashed() { + return crashed.get(); + } + /** * Stop nio server from processing requests. (for testing) */ @@ -166,6 +172,7 @@ public class NIOServerFactory extends Th LOG.warn("Exception in server socket loop: " + ss.socket().getInetAddress(), e); } catch (Throwable e) { LOG.error("Error in server socket loop: " + ss.socket().getInetAddress(), e); + crashed.set(true); break; } } Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieShutdownTest.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieShutdownTest.java?rev=1521028&r1=1521027&r2=1521028&view=diff ============================================================================== --- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieShutdownTest.java (original) +++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieShutdownTest.java Mon Sep 9 10:29:09 2013 @@ -25,7 +25,19 @@ import java.util.concurrent.TimeUnit; import org.apache.bookkeeper.conf.ServerConfiguration; import org.apache.bookkeeper.test.BookKeeperClusterTestCase; +import org.apache.bookkeeper.client.LedgerHandle; +import org.apache.bookkeeper.client.BookKeeper; +import org.apache.bookkeeper.client.AsyncCallback.AddCallback; + +import org.apache.bookkeeper.proto.BookieServer; +import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.WriteCallback; + +import java.io.IOException; +import org.apache.zookeeper.KeeperException; + +import java.nio.ByteBuffer; import org.junit.Test; +import org.junit.Assert; public class BookieShutdownTest extends BookKeeperClusterTestCase { @@ -68,4 +80,43 @@ public class BookieShutdownTest extends latch.countDown(); shutdownComplete.await(5000, TimeUnit.MILLISECONDS); } + + /** + * Test whether bookieserver returns the correct error code when it crashes. + */ + @Test(timeout=60000) + public void testBookieServerThreadError() throws Exception { + ServerConfiguration conf = bsConfs.get(0); + killBookie(0); + final CountDownLatch latch = new CountDownLatch(1); + final CountDownLatch shutdownComplete = new CountDownLatch(1); + // simulating ZooKeeper exception by assigning a closed zk client to bk + BookieServer bkServer = new BookieServer(conf) { + protected Bookie newBookie(ServerConfiguration conf) + throws IOException, KeeperException, InterruptedException, + BookieException { + return new Bookie(conf) { + @Override + public void addEntry(ByteBuffer entry, WriteCallback cb, + Object ctx, byte[] masterKey) + throws IOException, BookieException { + throw new OutOfMemoryError(); + } + }; + } + }; + bkServer.start(); + + LedgerHandle lh = bkc.createLedger(1, 1, BookKeeper.DigestType.CRC32, "passwd".getBytes()); + lh.asyncAddEntry("test".getBytes(), new AddCallback() { + @Override + public void addComplete(int rc, LedgerHandle lh, long entryId, Object ctx) { + // dont care, only trying to trigger OOM + } + }, null); + bkServer.join(); + Assert.assertFalse("Should have died", bkServer.isRunning()); + Assert.assertEquals("Should have died with server exception code", + ExitCode.SERVER_EXCEPTION, bkServer.getExitCode()); + } }