zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From iv...@apache.org
Subject svn commit: r1372343 - in /zookeeper/bookkeeper/trunk: CHANGES.txt bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java
Date Mon, 13 Aug 2012 09:38:00 GMT
Author: ivank
Date: Mon Aug 13 09:38:00 2012
New Revision: 1372343

URL: http://svn.apache.org/viewvc?rev=1372343&view=rev
Log:
BOOKKEEPER-326: DeadLock during ledger recovery (rakeshr via ivank)

Modified:
    zookeeper/bookkeeper/trunk/CHANGES.txt
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java

Modified: zookeeper/bookkeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1372343&r1=1372342&r2=1372343&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/CHANGES.txt (original)
+++ zookeeper/bookkeeper/trunk/CHANGES.txt Mon Aug 13 09:38:00 2012
@@ -50,6 +50,8 @@ Trunk (unreleased changes)
 
         BOOKKEEPER-349: Entry logger should close all the chennels which are there in Map,
instead of closing only current channel. (umamaheswararao via sijie)
 
+        BOOKKEEPER-326: DeadLock during ledger recovery (rakeshr via ivank)
+
       hedwig-client:
 
         BOOKKEEPER-274: Hedwig cpp client library should not link to cppunit which is just
used for test. (sijie via ivank)

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java?rev=1372343&r1=1372342&r2=1372343&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
Mon Aug 13 09:38:00 2012
@@ -114,13 +114,7 @@ public class PerChannelBookieClient exte
         this.readTimeoutTimer = null;
     }
 
-    synchronized private void connect() {
-        if (state == ConnectionState.CONNECTING) {
-            return;
-        } 
-        // Start the connection attempt to the input server host.
-        state = ConnectionState.CONNECTING;
-
+    private void connect() {
         if (LOG.isDebugEnabled())
             LOG.debug("Connecting to bookie: " + addr);
 
@@ -153,8 +147,6 @@ public class PerChannelBookieClient exte
                         state = ConnectionState.DISCONNECTED;
                     }
 
-                    PerChannelBookieClient.this.channel = channel;
-
                     // trick to not do operations under the lock, take the list
                     // of pending ops and assign it to a new variable, while
                     // emptying the pending ops by just assigning it to a new
@@ -171,7 +163,7 @@ public class PerChannelBookieClient exte
     }
 
     void connectIfNeededAndDoOp(GenericCallback<Void> op) {
-        boolean doOpNow;
+        boolean doOpNow = false;
 
         // common case without lock first
         if (channel != null && state == ConnectionState.CONNECTED) {
@@ -179,26 +171,29 @@ public class PerChannelBookieClient exte
         } else {
 
             synchronized (this) {
-                // check again under lock
+                // check the channel status again under lock
                 if (channel != null && state == ConnectionState.CONNECTED) {
                     doOpNow = true;
                 } else {
-
-                    // if reached here, channel is either null (first connection
-                    // attempt),
-                    // or the channel is disconnected
-                    doOpNow = false;
-
-                    // connection attempt is still in progress, queue up this
-                    // op. Op will be executed when connection attempt either
-                    // fails
-                    // or
-                    // succeeds
+                    // channel is either null (first connection attempt), or the
+                    // channel is disconnected. Connection attempt is still in
+                    // progress, queue up this op. Op will be executed when
+                    // connection attempt either fails or succeeds
                     pendingOps.add(op);
 
-                    connect();
+                    if (state == ConnectionState.CONNECTING) {
+                        // just return as connection request has already send
+                        // and waiting for the response.
+                        return;
+                    }
+                    // switch state to connecting and do connection attempt
+                    state = ConnectionState.CONNECTING;
                 }
             }
+            if (!doOpNow) {
+                // Start connection attempt to the input server host.
+                connect();
+            }
         }
 
         if (doOpNow) {

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java?rev=1372343&r1=1372342&r2=1372343&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java
Mon Aug 13 09:38:00 2012
@@ -357,4 +357,43 @@ public class BookieFailureTest extends M
         }
     }
 
+    /**
+     * Verify read last confirmed op, it shouldn't cause any deadlock as new
+     * bookie connection is being established and returning the connection
+     * notification in the same caller thread. It would be simulated by delaying
+     * the future.addlistener() in PerChannelBookieClient after the connection
+     * establishment. Now the future.addlistener() will notify back in the same
+     * thread and simultaneously invoke the pendingOp.operationComplete() event.
+     * 
+     * BOOKKEEPER-326
+     */
+    @Test
+    public void testReadLastConfirmedOp() throws Exception {
+        startNewBookie();
+        startNewBookie();
+        // Create a ledger
+        LedgerHandle beforelh = bkc.createLedger(numBookies + 2,
+                numBookies + 2, digestType, "".getBytes());
+
+        int numEntries = 10;
+        String tmp = "BookKeeper is cool!";
+        for (int i = 0; i < numEntries; i++) {
+            beforelh.addEntry(tmp.getBytes());
+        }
+
+        // shutdown first bookie server
+        killBookie(0);
+        startNewBookie();
+
+        // create new bookie client, and forcing to establish new
+        // PerChannelBookieClient connections for recovery flows.
+        BookKeeperTestClient bkc1 = new BookKeeperTestClient(baseClientConf);
+        // try to open ledger with recovery
+        LedgerHandle afterlh = bkc1.openLedger(beforelh.getId(), digestType, ""
+                .getBytes());
+
+        assertEquals("Entries got missed", beforelh.getLastAddPushed(), afterlh
+                .getLastAddConfirmed());
+        bkc1.close();
+    }
 }



Mime
View raw message