You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by iv...@apache.org on 2012/03/08 19:24:51 UTC

svn commit: r1298492 - 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/test/java/org/apache/bookkeeper/client/ bookkeepe...

Author: ivank
Date: Thu Mar  8 18:24:51 2012
New Revision: 1298492

URL: http://svn.apache.org/viewvc?rev=1298492&view=rev
Log:
BOOKKEEPER-180: bookie server doesn't quit when running out of disk space (sijie via ivank)

Added:
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ExitCode.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/proto/BookieServer.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ConcurrentLedgerTest.java
    zookeeper/bookkeeper/trunk/hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java

Modified: zookeeper/bookkeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1298492&r1=1298491&r2=1298492&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/CHANGES.txt (original)
+++ zookeeper/bookkeeper/trunk/CHANGES.txt Thu Mar  8 18:24:51 2012
@@ -50,6 +50,8 @@ Trunk (unreleased changes)
 
 	BOOKKEEPER-176: HierarchicalBookieFailureTest Hung (ivank via fpj)
 
+        BOOKKEEPER-180: bookie server doesn't quit when running out of disk space (sijie via ivank)
+
       hedwig-server/
       
         BOOKKEEPER-140: Hub server doesn't subscribe remote region correctly when a region is down. (Sijie Gou via ivank)

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=1298492&r1=1298491&r2=1298492&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 Thu Mar  8 18:24:51 2012
@@ -67,6 +67,7 @@ import org.apache.zookeeper.ZooDefs.Ids;
 public class Bookie extends Thread {
     HashMap<Long, LedgerDescriptor> ledgers = new HashMap<Long, LedgerDescriptor>();
     static Logger LOG = LoggerFactory.getLogger(Bookie.class);
+
     final static long MB = 1024 * 1024L;
     // max journal file size
     final long maxJournalSize;
@@ -102,6 +103,10 @@ public class Bookie extends Thread {
 
     // Running flag
     private volatile boolean running = false;
+    // Flag identify whether it is in shutting down progress
+    private volatile boolean shuttingdown = false;
+
+    private int exitCode = ExitCode.OK;
 
     // jmx related beans
     BookieBean jmxBookieBean;
@@ -428,12 +433,7 @@ public class Bookie extends Thread {
             registerBookie(conf.getBookiePort());
         } catch (IOException e) {
             LOG.error("Couldn't register bookie with zookeeper, shutting down", e);
-            try {
-                shutdown();
-            } catch (InterruptedException ie) {
-                LOG.error("Interrupted shutting down", ie);
-                System.exit(-1);
-            }
+            shutdown(ExitCode.ZK_REG_FAIL);
         }
     }
 
@@ -594,11 +594,7 @@ public class Bookie extends Thread {
                 Watcher.Event.KeeperState.Expired)) {
                     LOG.error("ZK client connection to the ZK server has expired!");
                     isZkExpired = true;
-                    try {
-                        shutdown();
-                    } catch (InterruptedException ie) {
-                        System.exit(-1);
-                    }
+                    shutdown(ExitCode.ZK_EXPIRED);
                 }
             }
         });
@@ -928,31 +924,56 @@ public class Bookie extends Thread {
                 qe = null;
             }
         } catch (Exception e) {
-            LOG.error("Bookie thread exiting", e);
+            // if the bookie thread quits due to shutting down, it is ok
+            if (shuttingdown) {
+                LOG.warn("Bookie thread exits when shutting down", e);
+            } else {
+                // some error found in bookie thread and it quits
+                // following add operations to it would hang unit client timeout
+                // so we should let bookie server exists
+                LOG.error("Exception occurred in bookie thread and it quits : ", e);
+                shutdown(ExitCode.BOOKIE_EXCEPTION);
+            }
         }
     }
 
-    public synchronized void shutdown() throws InterruptedException {
-        if (!running) { // avoid shutdown twice
-            return;
+    // provided a public shutdown method for other caller
+    // to shut down bookie gracefully
+    public int shutdown() {
+        return shutdown(ExitCode.OK);
+    }
+
+    // internal shutdown method to let shutdown bookie gracefully
+    // when encountering exception
+    synchronized int shutdown(int exitCode) {
+        try {
+            if (running) { // avoid shutdown twice
+                // the exitCode only set when first shutdown usually due to exception found
+                this.exitCode = exitCode;
+                // mark bookie as in shutting down progress
+                shuttingdown = true;
+                // shut down gc thread, which depends on zookeeper client
+                // also compaction will write entries again to entry log file
+                gcThread.shutdown();
+                // Shutdown the ZK client
+                if(zk != null) zk.close();
+                this.interrupt();
+                this.join();
+                syncThread.shutdown();
+                for(LedgerDescriptor d: ledgers.values()) {
+                    d.close();
+                }
+                // Shutdown the EntryLogger which has the GarbageCollector Thread running
+                entryLogger.shutdown();
+                // close Ledger Manager
+                ledgerManager.close();
+                // setting running to false here, so watch thread in bookie server know it only after bookie shut down
+                running = false;
+            }
+        } catch (InterruptedException ie) {
+            LOG.error("Interrupted during shutting down bookie : ", ie);
         }
-        // shut down gc thread, which depends on zookeeper client
-        // also compaction will write entries again to entry log file
-        gcThread.shutdown();
-        // Shutdown the ZK client
-        if(zk != null) zk.close();
-        this.interrupt();
-        this.join();
-        syncThread.shutdown(); 
-        for(LedgerDescriptor d: ledgers.values()) {
-            d.close();
-        }
-        // Shutdown the EntryLogger which has the GarbageCollector Thread running
-        entryLogger.shutdown();
-        // close Ledger Manager
-        ledgerManager.close();
-        // setting running to false here, so watch thread in bookie server know it only after bookie shut down
-        running = false;
+        return exitCode;
     }
 
     /** 

Added: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ExitCode.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ExitCode.java?rev=1298492&view=auto
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ExitCode.java (added)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ExitCode.java Thu Mar  8 18:24:51 2012
@@ -0,0 +1,40 @@
+/*
+ *
+ * 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;
+
+/**
+ * Exit code used to exit bookie server
+ */
+public class ExitCode {
+    // normal quit
+    public final static int OK                  = 0;
+    // invalid configuration
+    public final static int INVALID_CONF        = 1;
+    // exception running bookie server
+    public final static int SERVER_EXCEPTION    = 2;
+    // zookeeper is expired
+    public final static int ZK_EXPIRED          = 3;
+    // register bookie on zookeeper failed
+    public final static int ZK_REG_FAIL         = 4;
+    // exception running bookie
+    public final static int BOOKIE_EXCEPTION    = 5;
+}

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=1298492&r1=1298491&r2=1298492&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 Thu Mar  8 18:24:51 2012
@@ -35,6 +35,7 @@ import org.apache.zookeeper.KeeperExcept
 
 import org.apache.bookkeeper.bookie.Bookie;
 import org.apache.bookkeeper.bookie.BookieException;
+import org.apache.bookkeeper.bookie.ExitCode;
 import org.apache.bookkeeper.conf.ServerConfiguration;
 import org.apache.bookkeeper.jmx.BKMBeanRegistry;
 import org.apache.bookkeeper.proto.NIOServerFactory.Cnxn;
@@ -61,6 +62,8 @@ 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;
@@ -94,12 +97,12 @@ public class BookieServer implements NIO
         }
     }
 
-    public synchronized void shutdown() throws InterruptedException {
+    public synchronized void shutdown() {
         if (!running) {
             return;
         }
         nioServerFactory.shutdown();
-        bookie.shutdown();
+        exitCode = bookie.shutdown();
         running = false;
 
         // unregister JMX
@@ -156,6 +159,10 @@ public class BookieServer implements NIO
         nioServerFactory.join();
     }
 
+    public int getExitCode() {
+        return exitCode;
+    }
+
     /**
      * A thread to watch whether bookie & nioserver is still alive
      */
@@ -176,11 +183,7 @@ public class BookieServer implements NIO
                     // do nothing
                 }
                 if (!isBookieRunning() || !isNioServerRunning()) {
-                    try {
-                        shutdown();
-                    } catch (InterruptedException ie) {
-                        System.exit(-1);
-                    }
+                    shutdown();
                     break;
                 }
             }
@@ -262,8 +265,7 @@ public class BookieServer implements NIO
      * @throws IOException
      * @throws InterruptedException
      */
-    public static void main(String[] args) 
-            throws IOException, KeeperException, InterruptedException, BookieException {
+    public static void main(String[] args) {
         ServerConfiguration conf = null;
         try {
             conf = parseArgs(args);
@@ -271,7 +273,7 @@ public class BookieServer implements NIO
             LOG.error("Error parsing command line arguments : ", iae);
             System.err.println(iae.getMessage());
             printUsage();
-            throw iae;
+            System.exit(ExitCode.INVALID_CONF);
         }
 
         StringBuilder sb = new StringBuilder();
@@ -288,21 +290,24 @@ public class BookieServer implements NIO
                            conf.getBookiePort(), conf.getZkServers(),
                            conf.getJournalDirName(), sb);
         LOG.info(hello);
-        final BookieServer bs = new BookieServer(conf);
-        bs.start();
-        Runtime.getRuntime().addShutdownHook(new Thread() {
-            @Override
-            public void run() {
-                try {
+        try {
+            final BookieServer bs = new BookieServer(conf);
+            bs.start();
+            Runtime.getRuntime().addShutdownHook(new Thread() {
+                @Override
+                public void run() {
                     bs.shutdown();
                     LOG.info("Shut down bookie server successfully");
-                } catch (InterruptedException ie) {
-                    LOG.warn("Exception when shutting down bookie server : ", ie);
                 }
-            }
-        });
-        LOG.info("Register shutdown hook successfully");
-        bs.join();
+            });
+            LOG.info("Register shutdown hook successfully");
+            bs.join();
+
+            System.exit(bs.getExitCode());
+        } catch (Exception e) {
+            LOG.error("Exception running bookie server : ", e);
+            System.exit(ExitCode.SERVER_EXCEPTION);
+        }
     }
 
     public void processPacket(ByteBuffer packet, Cnxn src) {

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java?rev=1298492&r1=1298491&r2=1298492&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java Thu Mar  8 18:24:51 2012
@@ -93,13 +93,9 @@ public class LedgerCacheTest extends Tes
     @Override
     @After
     public void tearDown() {
-        try {
-            bookie.shutdown();
-            recursiveDelete(txnDir);
-            recursiveDelete(ledgerDir);
-        } catch (InterruptedException e) {
-            LOG.error("Error tearing down", e);
-        }
+        bookie.shutdown();
+        recursiveDelete(txnDir);
+        recursiveDelete(ledgerDir);
     }
 
     /**

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ConcurrentLedgerTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ConcurrentLedgerTest.java?rev=1298492&r1=1298491&r2=1298492&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ConcurrentLedgerTest.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ConcurrentLedgerTest.java Thu Mar  8 18:24:51 2012
@@ -93,13 +93,9 @@ public class ConcurrentLedgerTest extend
     @Override
     @After
     public void tearDown() {
-        try {
-            bookie.shutdown();
-            recursiveDelete(txnDir);
-            recursiveDelete(ledgerDir);
-        } catch (InterruptedException e) {
-            LOG.error("Error tearing down", e);
-        }
+        bookie.shutdown();
+        recursiveDelete(txnDir);
+        recursiveDelete(ledgerDir);
     }
 
     byte zeros[] = new byte[16];

Modified: zookeeper/bookkeeper/trunk/hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java?rev=1298492&r1=1298491&r2=1298492&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java (original)
+++ zookeeper/bookkeeper/trunk/hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java Thu Mar  8 18:24:51 2012
@@ -121,12 +121,8 @@ public class BookKeeperTestBase extends 
     @After
     public void tearDown() throws Exception {
         // Shutdown all of the bookie servers
-        try {
-            for (BookieServer bs : bookiesList) {
-                bs.shutdown();
-            }
-        } catch (InterruptedException e) {
-            LOG.error("Error tearing down", e);
+        for (BookieServer bs : bookiesList) {
+            bs.shutdown();
         }
         // Close the BookKeeper client
         bk.close();
@@ -134,23 +130,15 @@ public class BookKeeperTestBase extends 
     }
 
     public void stopAllBookieServers() throws Exception {
-        try {
-            for (BookieServer bs : bookiesList) {
-                bs.shutdown();
-            }
-            bookiesList.clear();
-        } catch (InterruptedException e) {
-            LOG.error("Error stopping all bookie servers", e);
+        for (BookieServer bs : bookiesList) {
+            bs.shutdown();
         }
+        bookiesList.clear();
     }
 
     public void startAllBookieServers() throws Exception {
-        try {
-            for (ServerConfiguration conf : bkConfsList) {
-                bookiesList.add(startBookie(conf));
-            }
-        } catch (InterruptedException e) {
-            LOG.error("Error starting all bookie servers", e);
+        for (ServerConfiguration conf : bkConfsList) {
+            bookiesList.add(startBookie(conf));
         }
     }
     
@@ -158,11 +146,7 @@ public class BookKeeperTestBase extends 
         Random r = new Random();
         int bi = r.nextInt(bookiesList.size());
         BookieServer bs = bookiesList.get(bi);
-        try {
-            bs.shutdown();
-        } catch (InterruptedException e) {
-            LOG.error("Error tearing down", e);
-        }
+        bs.shutdown();
         bookiesList.remove(bi);
         bkConfsList.remove(bi);
     }