Return-Path: X-Original-To: apmail-zookeeper-commits-archive@www.apache.org Delivered-To: apmail-zookeeper-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 7F462102EA for ; Fri, 6 Dec 2013 06:40:15 +0000 (UTC) Received: (qmail 65340 invoked by uid 500); 6 Dec 2013 06:40:14 -0000 Delivered-To: apmail-zookeeper-commits-archive@zookeeper.apache.org Received: (qmail 65011 invoked by uid 500); 6 Dec 2013 06:40:07 -0000 Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ Delivered-To: mailing list commits@zookeeper.apache.org Received: (qmail 64982 invoked by uid 99); 6 Dec 2013 06:39:59 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 06 Dec 2013 06:39:59 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 06 Dec 2013 06:39:57 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 7AE3F23889E3; Fri, 6 Dec 2013 06:39:37 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1548385 - in /zookeeper/bookkeeper/trunk: ./ bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/ bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ book... Date: Fri, 06 Dec 2013 06:39:37 -0000 To: commits@zookeeper.apache.org From: sijie@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20131206063937.7AE3F23889E3@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: sijie Date: Fri Dec 6 06:39:36 2013 New Revision: 1548385 URL: http://svn.apache.org/r1548385 Log: BOOKKEEPER-701: Improve exception handling of Bookkeeper threads (rakesh via sijie) Added: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieCriticalThread.java zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieThreadTest.java Modified: zookeeper/bookkeeper/trunk/CHANGES.txt zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieThread.java zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AutoRecoveryMain.java zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java Modified: zookeeper/bookkeeper/trunk/CHANGES.txt URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1548385&r1=1548384&r2=1548385&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/CHANGES.txt (original) +++ zookeeper/bookkeeper/trunk/CHANGES.txt Fri Dec 6 06:39:36 2013 @@ -200,6 +200,8 @@ Trunk (unreleased changes) BOOKKEEPER-699: Codahale metrics implementation of stats API (ivank via sijie) + BOOKKEEPER-701: Improve exception handling of Bookkeeper threads (rakesh via sijie) + NEW FEATURE: BOOKKEEPER-562: Ability to tell if a ledger is closed or not (fpj) Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java?rev=1548385&r1=1548384&r2=1548385&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java Fri Dec 6 06:39:36 2013 @@ -31,10 +31,7 @@ import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.nio.ByteBuffer; import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CountDownLatch; @@ -51,7 +48,6 @@ import org.apache.bookkeeper.jmx.BKMBean import org.apache.bookkeeper.jmx.BKMBeanRegistry; import org.apache.bookkeeper.meta.LedgerManager; import org.apache.bookkeeper.meta.LedgerManagerFactory; -import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.WriteCallback; import org.apache.bookkeeper.util.BookKeeperConstants; import org.apache.bookkeeper.util.IOUtils; @@ -79,7 +75,7 @@ import com.google.common.annotations.Vis * */ -public class Bookie extends BookieThread { +public class Bookie extends BookieCriticalThread { private final static Logger LOG = LoggerFactory.getLogger(Bookie.class); @@ -835,11 +831,12 @@ public class Bookie extends BookieThread } LOG.info("Triggering shutdown of Bookie-{} with exitCode {}", conf.getBookiePort(), exitCode); - new Thread("BookieShutdownTrigger") { + BookieThread th = new BookieThread("BookieShutdownTrigger") { public void run() { Bookie.this.shutdown(exitCode); } - }.start(); + }; + th.start(); } // provided a public shutdown method for other caller Added: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieCriticalThread.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieCriticalThread.java?rev=1548385&view=auto ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieCriticalThread.java (added) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieCriticalThread.java Fri Dec 6 06:39:36 2013 @@ -0,0 +1,45 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.bookkeeper.bookie; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Thread is marked as critical and will exit, when there is an uncaught + * exception occurred in thread. + */ +public class BookieCriticalThread extends BookieThread { + private static final Logger LOG = LoggerFactory + .getLogger(BookieCriticalThread.class); + + public BookieCriticalThread(String name) { + super(name); + } + + public BookieCriticalThread(Runnable thread, String name) { + super(thread, name); + } + + @Override + protected void handleException(Thread t, Throwable e) { + LOG.error("Uncaught exception in thread {} and is exiting!", + t.getName(), e); + Runtime.getRuntime().exit(ExitCode.BOOKIE_EXCEPTION); + } +} Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieThread.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieThread.java?rev=1548385&r1=1548384&r2=1548385&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieThread.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieThread.java Fri Dec 6 06:39:36 2013 @@ -25,21 +25,31 @@ import org.slf4j.LoggerFactory; * Any common handing that we require for all bookie threads * should be implemented here */ -public class BookieThread extends Thread { +public class BookieThread extends Thread implements + Thread.UncaughtExceptionHandler { - private static class BookieUncaughtExceptionHandler implements Thread.UncaughtExceptionHandler { - static Logger logger = LoggerFactory.getLogger(BookieUncaughtExceptionHandler.class); - - @Override - public void uncaughtException(Thread t, Throwable e) { - logger.error("Uncaught exception in thread " + t.getName(), e); - Runtime.getRuntime().exit(1); - } + private static final Logger LOG = LoggerFactory + .getLogger(BookieThread.class); + @Override + public void uncaughtException(Thread t, Throwable e) { + handleException(t, e); } - public BookieThread (String name) { + public BookieThread(String name) { super(name); - setUncaughtExceptionHandler(new BookieUncaughtExceptionHandler()); + setUncaughtExceptionHandler(this); + } + + public BookieThread(Runnable thread, String name) { + super(thread, name); + setUncaughtExceptionHandler(this); + } + + /** + * Handles uncaught exception occurred in thread + */ + protected void handleException(Thread t, Throwable e) { + LOG.error("Uncaught exception in thread {}", t.getName(), e); } } Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java?rev=1548385&r1=1548384&r2=1548385&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java Fri Dec 6 06:39:36 2013 @@ -47,7 +47,7 @@ import org.slf4j.LoggerFactory; * This is the garbage collector thread that runs in the background to * remove any entry log files that no longer contains any active ledger. */ -public class GarbageCollectorThread extends Thread { +public class GarbageCollectorThread extends BookieThread { private static final Logger LOG = LoggerFactory.getLogger(GarbageCollectorThread.class); private static final int SECOND = 1000; Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java?rev=1548385&r1=1548384&r2=1548385&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java Fri Dec 6 06:39:36 2013 @@ -47,7 +47,7 @@ import org.slf4j.LoggerFactory; /** * Provide journal related management. */ -class Journal extends BookieThread implements CheckpointSource { +class Journal extends BookieCriticalThread implements CheckpointSource { private final static Logger LOG = LoggerFactory.getLogger(Journal.class); @@ -286,7 +286,7 @@ class Journal extends BookieThread imple } } - private class ForceWriteRequest implements Runnable { + private class ForceWriteRequest extends BookieCriticalThread { private final JournalChannel logFile; private final LinkedList forceWriteWaiters; private boolean shouldClose; @@ -300,6 +300,7 @@ class Journal extends BookieThread imple LinkedList forceWriteWaiters, boolean shouldClose, boolean isMarker) { + super("ForceWriteRequestThread"); this.forceWriteWaiters = forceWriteWaiters; this.logFile = logFile; this.logId = logId; @@ -357,7 +358,7 @@ class Journal extends BookieThread imple * ForceWriteThread is a background thread which makes the journal durable periodically * */ - private class ForceWriteThread extends BookieThread { + private class ForceWriteThread extends BookieCriticalThread { volatile boolean running = true; // This holds the queue entries that should be notified after a // successful force write Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java?rev=1548385&r1=1548384&r2=1548385&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java Fri Dec 6 06:39:36 2013 @@ -186,7 +186,7 @@ public class LedgerDirsManager { /** * Thread to monitor the disk space periodically. */ - private class LedgerDirsMonitor extends Thread { + private class LedgerDirsMonitor extends BookieThread { private final int interval; public LedgerDirsMonitor(int interval) { Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java?rev=1548385&r1=1548384&r2=1548385&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java Fri Dec 6 06:39:36 2013 @@ -29,6 +29,7 @@ import java.net.UnknownHostException; import org.apache.zookeeper.KeeperException; import org.apache.bookkeeper.bookie.Bookie; +import org.apache.bookkeeper.bookie.BookieCriticalThread; import org.apache.bookkeeper.bookie.BookieException; import org.apache.bookkeeper.bookie.ExitCode; import org.apache.bookkeeper.conf.ServerConfiguration; @@ -205,7 +206,7 @@ public class BookieServer { /** * A thread to watch whether bookie & nioserver is still alive */ - private class DeathWatcher extends Thread { + private class DeathWatcher extends BookieCriticalThread { private final int watchInterval; Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AutoRecoveryMain.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AutoRecoveryMain.java?rev=1548385&r1=1548384&r2=1548385&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AutoRecoveryMain.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AutoRecoveryMain.java Fri Dec 6 06:39:36 2013 @@ -25,6 +25,7 @@ import java.io.IOException; import java.net.MalformedURLException; import org.apache.bookkeeper.bookie.Bookie; +import org.apache.bookkeeper.bookie.BookieCriticalThread; import org.apache.bookkeeper.bookie.ExitCode; import org.apache.bookkeeper.conf.ServerConfiguration; import org.apache.bookkeeper.replication.ReplicationException.CompatibilityException; @@ -149,7 +150,7 @@ public class AutoRecoveryMain { /* * DeathWatcher for AutoRecovery daemons. */ - private static class AutoRecoveryDeathWatcher extends Thread { + private static class AutoRecoveryDeathWatcher extends BookieCriticalThread { private int watchInterval; private AutoRecoveryMain autoRecoveryMain; Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java?rev=1548385&r1=1548384&r2=1548385&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java Fri Dec 6 06:39:36 2013 @@ -27,6 +27,7 @@ import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.CountDownLatch; +import org.apache.bookkeeper.bookie.BookieThread; import org.apache.bookkeeper.client.BKException; import org.apache.bookkeeper.client.BookKeeper; import org.apache.bookkeeper.client.BookKeeperAdmin; @@ -94,7 +95,7 @@ public class ReplicationWorker implement this.bkc = new BookKeeper(new ClientConfiguration(conf), zkc); this.admin = new BookKeeperAdmin(bkc); this.ledgerChecker = new LedgerChecker(bkc); - this.workerThread = new Thread(this, "ReplicationWorker"); + this.workerThread = new BookieThread(this, "ReplicationWorker"); this.openLedgerRereplicationGracePeriod = conf .getOpenLedgerRereplicationGracePeriod(); this.pendingReplicationTimer = new Timer("PendingReplicationTimer"); @@ -322,7 +323,7 @@ public class ReplicationWorker implement * Gives the running status of ReplicationWorker */ boolean isRunning() { - return workerRunning; + return workerRunning && workerThread.isAlive(); } private boolean isTargetBookieExistsInFragmentEnsemble(LedgerHandle lh, Added: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieThreadTest.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieThreadTest.java?rev=1548385&view=auto ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieThreadTest.java (added) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieThreadTest.java Fri Dec 6 06:39:36 2013 @@ -0,0 +1,84 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.bookkeeper.bookie; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.junit.Assert; +import org.junit.Test; + +/** + * Testing bookie thread cases + */ +public class BookieThreadTest { + + private CountDownLatch runningLatch = new CountDownLatch(1); + + public class MyThread extends BookieThread { + + public MyThread(String threadName) { + super(threadName); + } + + public void run() { + throw new Error(); + } + + @Override + protected void handleException(Thread t, Throwable e) { + Assert.assertEquals("Unknown thread!", this, t); + runningLatch.countDown(); + } + } + + public class MyCriticalThread extends BookieCriticalThread { + + public MyCriticalThread(String threadName) { + super(threadName); + } + + public void run() { + throw new Error(); + } + + @Override + protected void handleException(Thread t, Throwable e) { + Assert.assertEquals("Unknown thread!", this, t); + runningLatch.countDown(); + } + } + + /** + * Test verifies uncaught exception handling of BookieThread + */ + @Test(timeout = 30000) + public void testUncaughtException() throws Exception { + MyThread myThread = new MyThread("Test-Thread"); + myThread.start(); + Assert.assertTrue("Uncaught exception is not properly handled.", + runningLatch.await(10000, TimeUnit.MILLISECONDS)); + + runningLatch = new CountDownLatch(1); + MyCriticalThread myCriticalThread = new MyCriticalThread( + "Test-Critical-Thread"); + myCriticalThread.start(); + Assert.assertTrue("Uncaught exception is not properly handled.", + runningLatch.await(10000, TimeUnit.MILLISECONDS)); + } +}