Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@<zookeeper.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1495514 - in /zookeeper/bookkeeper/branches/branch-4.2:
 CHANGES.txt
 bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
 bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieShutdownTest.java
Date: Fri, 21 Jun 2013 17:34:10 -0000
To: commits@zookeeper.apache.org
From: ivank@apache.org
Message-Id: <20130621173410.5BC122388C7E@eris.apache.org>

Author: ivank
Date: Fri Jun 21 17:34:09 2013
New Revision: 1495514

URL: http://svn.apache.org/r1495514
Log:
BOOKKEEPER-313: Bookkeeper shutdown call from Bookie thread is not shutting down server (vinay via ivank)

Added:
    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
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.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=1495514&r1=1495513&r2=1495514&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt Fri Jun 21 17:34:09 2013
@@ -27,6 +27,7 @@ Release 4.2.2 - Unreleased
 	BOOKKEEPER-626: BOOKIE_EXTRA_OPTS are added twice (vinay via fpj)
 
         BOOKKEEPER-619: Bookie should not create local cookie files if zookeeper is uninitialized (ivank)
+        BOOKKEEPER-313: Bookkeeper shutdown call from Bookie thread is not shutting down server (vinay via ivank)
 
       hedwig-server:
 

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java?rev=1495514&r1=1495513&r2=1495514&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java Fri Jun 21 17:34:09 2013
@@ -791,6 +791,10 @@ public class Bookie extends Thread {
      */
     @VisibleForTesting
     public void transitionToReadOnlyMode() {
+        if (shuttingdown == true) {
+            return;
+        }
+
         if (!readOnly.compareAndSet(false, true)) {
             return;
         }
@@ -910,19 +914,18 @@ public class Bookie extends Thread {
     // Triggering the Bookie shutdown in its own thread,
     // because shutdown can be called from sync thread which would be
     // interrupted by shutdown call.
+    AtomicBoolean shutdownTriggered = new AtomicBoolean(false);
     void triggerBookieShutdown(final int exitCode) {
-        Thread shutdownThread = new Thread() {
+        if (!shutdownTriggered.compareAndSet(false, true)) {
+            return;
+        }
+        LOG.info("Triggering shutdown of Bookie-{} with exitCode {}",
+                 conf.getBookiePort(), exitCode);
+        new Thread("BookieShutdownTrigger") {
             public void run() {
                 Bookie.this.shutdown(exitCode);
             }
-        };
-        shutdownThread.start();
-        try {
-            shutdownThread.join();
-        } catch (InterruptedException e) {
-            Thread.currentThread().interrupt();
-            LOG.debug("InterruptedException while waiting for shutdown. Not a problem!!");
-        }
+        }.start();
     }
 
     // provided a public shutdown method for other caller
@@ -937,16 +940,12 @@ public class Bookie extends Thread {
         try {
             if (running) { // avoid shutdown twice
                 // the exitCode only set when first shutdown usually due to exception found
+                LOG.info("Shutting down Bookie-{} with exitCode {}",
+                         conf.getBookiePort(), exitCode);
                 this.exitCode = exitCode;
                 // mark bookie as in shutting down progress
                 shuttingdown = true;
 
-                // Shutdown the ZK client
-                if(zk != null) zk.close();
-
-                //Shutdown disk checker
-                ledgerDirsManager.shutdown();
-
                 // Shutdown journal
                 journal.shutdown();
                 this.join();
@@ -962,7 +961,15 @@ public class Bookie extends Thread {
                 } catch (IOException ie) {
                     LOG.error("Failed to close active ledger manager : ", ie);
                 }
-                // setting running to false here, so watch thread in bookie server know it only after bookie shut down
+
+                //Shutdown disk checker
+                ledgerDirsManager.shutdown();
+
+                // Shutdown the ZK client
+                if(zk != null) zk.close();
+
+                // setting running to false here, so watch thread
+                // in bookie server know it only after bookie shut down
                 running = false;
             }
         } catch (InterruptedException ie) {

Added: 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=1495514&view=auto
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieShutdownTest.java (added)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieShutdownTest.java Fri Jun 21 17:34:09 2013
@@ -0,0 +1,71 @@
+/*
+ *
+ * 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.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
+import org.junit.Test;
+
+public class BookieShutdownTest extends BookKeeperClusterTestCase {
+
+    public BookieShutdownTest() {
+        super(1);
+    }
+
+    /**
+     * Test whether Bookie can be shutdown when the call comes inside bookie thread.
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void testBookieShutdownFromBookieThread() throws Exception {
+        ServerConfiguration conf = bsConfs.get(0);
+        killBookie(0);
+        final CountDownLatch latch = new CountDownLatch(1);
+        final CountDownLatch shutdownComplete = new CountDownLatch(1);
+        Bookie bookie = new Bookie(conf) {
+            @Override
+            public void run() {
+                try {
+                    latch.await();
+                } catch (InterruptedException e) {
+                    // Ignore
+                }
+                triggerBookieShutdown(ExitCode.BOOKIE_EXCEPTION);
+            }
+
+            @Override
+            synchronized int shutdown(int exitCode) {
+                super.shutdown(exitCode);
+                shutdownComplete.countDown();
+                return exitCode;
+            }
+        };
+        bookie.start();
+        // after 1 sec stop .
+        Thread.sleep(1000);
+        latch.countDown();
+        shutdownComplete.await(5000, TimeUnit.MILLISECONDS);
+    }
+}