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/02/13 15:46:38 UTC

svn commit: r1243539 - in /zookeeper/bookkeeper/trunk: ./ bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/ bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ bookkeepe...

Author: ivank
Date: Mon Feb 13 14:46:38 2012
New Revision: 1243539

URL: http://svn.apache.org/viewvc?rev=1243539&view=rev
Log:
BOOKKEEPER-152: Can't recover a ledger whose current ensemble contain failed bookie. (ivank)

Modified:
    zookeeper/bookkeeper/trunk/CHANGES.txt
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RoundRobinDistributionSchedule.java
    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/client/BookieRecoveryTest.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerRecoveryTest.java

Modified: zookeeper/bookkeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/CHANGES.txt (original)
+++ zookeeper/bookkeeper/trunk/CHANGES.txt Mon Feb 13 14:46:38 2012
@@ -34,6 +34,8 @@ Trunk (unreleased changes)
 
 	BOOKKEEPER-162: LedgerHandle.readLastConfirmed does not work (fpj)
 
+        BOOKKEEPER-152: Can't recover a ledger whose current ensemble contain failed bookie. (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/client/DigestManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java Mon Feb 13 14:46:38 2012
@@ -164,10 +164,12 @@ abstract class DigestManager {
     static class RecoveryData {
         long lastAddConfirmed;
         long entryId;
+        long length;
 
-        public RecoveryData(long lastAddConfirmed, long entryId) {
+        public RecoveryData(long lastAddConfirmed, long entryId, long length) {
             this.lastAddConfirmed = lastAddConfirmed;
             this.entryId = entryId;
+            this.length = length;
         }
 
     }
@@ -179,7 +181,6 @@ abstract class DigestManager {
         long entryId = dataReceived.readLong();
         long lastAddConfirmed = dataReceived.readLong();
         long length = dataReceived.readLong();
-        return new RecoveryData(lastAddConfirmed, entryId);
-
+        return new RecoveryData(lastAddConfirmed, entryId, length);
     }
 }

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java Mon Feb 13 14:46:38 2012
@@ -48,14 +48,18 @@ interface DistributionSchedule {
     public int getReplicaIndex(long entryId, int bookieIndex);
 
     /**
-     * Specifies whether its ok to proceed with recovery given that we have
-     * heard back from the given bookie index. These calls will be a made in a
-     * sequence and an implementation of this interface should accumulate
-     * history about which bookie indexes we have heard from. Once this method
-     * has returned true, it wont be called again on the same instance
-     *
-     * @param bookieIndexHeardFrom
-     * @return true if its ok to proceed with recovery
+     * Interface to keep track of which bookies in an ensemble, an action
+     * has been performed for.
      */
-    public boolean canProceedWithRecovery(int bookieIndexHeardFrom);
+    public interface QuorumCoverageSet {
+        /**
+         * Add a bookie to the set, and check if all quorum in the set
+         * have had the action performed for it.
+         * @param bookieIndexHeardFrom Bookie we've just heard from
+         * @return whether all quorums have been covered
+         */
+        public boolean addBookieAndCheckCovered(int bookieIndexHeardFrom);
+    }
+
+    public QuorumCoverageSet getCoverageSet();
 }

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java Mon Feb 13 14:46:38 2012
@@ -498,8 +498,20 @@ public class LedgerHandle {
      * @param ctx
      */
 
-    public void asyncReadLastConfirmed(ReadLastConfirmedCallback cb, Object ctx) {
-        new ReadLastConfirmedOp(this, cb, ctx).initiate();
+    public void asyncReadLastConfirmed(final ReadLastConfirmedCallback cb, final Object ctx) {
+        ReadLastConfirmedOp.LastConfirmedDataCallback innercb = new ReadLastConfirmedOp.LastConfirmedDataCallback() {
+                public void readLastConfirmedDataComplete(int rc, DigestManager.RecoveryData data) {
+                    if (rc == BKException.Code.OK) {
+                        lastAddConfirmed = Math.max(lastAddConfirmed, data.lastAddConfirmed);
+                        lastAddPushed = Math.max(lastAddPushed, data.lastAddConfirmed);
+                        length = Math.max(length, data.length);
+                        cb.readLastConfirmedComplete(rc, data.lastAddConfirmed, ctx);
+                    } else {
+                        cb.readLastConfirmedComplete(rc, -1, ctx);
+                    }
+                }
+            };
+        new ReadLastConfirmedOp(this, innercb).initiate();
     }
 
 
@@ -688,6 +700,13 @@ public class LedgerHandle {
             return;
         }
 
+        // if metadata is already in recover, dont try to write again,
+        // just do the recovery from the starting point
+        if (metadata.isInRecovery()) {
+            new LedgerRecoveryOp(LedgerHandle.this, cb).initiate();
+            return;
+        }
+
         metadata.markLedgerInRecovery();
 
         writeLedgerConfig(new StatCallback() {

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java Mon Feb 13 14:46:38 2012
@@ -23,11 +23,11 @@ import java.util.Enumeration;
 import org.apache.bookkeeper.client.AsyncCallback.AddCallback;
 import org.apache.bookkeeper.client.AsyncCallback.CloseCallback;
 import org.apache.bookkeeper.client.AsyncCallback.ReadCallback;
+import org.apache.bookkeeper.client.AsyncCallback.ReadLastConfirmedCallback;
 import org.apache.bookkeeper.client.BKException.BKDigestMatchException;
 import org.apache.bookkeeper.client.LedgerHandle.NoopCloseCallback;
 import org.apache.bookkeeper.client.DigestManager.RecoveryData;
 import org.apache.bookkeeper.proto.BookieProtocol;
-import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.ReadEntryCallback;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback;
 
 import org.apache.zookeeper.KeeperException;
@@ -43,7 +43,7 @@ import org.jboss.netty.buffer.ChannelBuf
  * the ledger at that entry.
  *
  */
-class LedgerRecoveryOp implements ReadEntryCallback, ReadCallback, AddCallback {
+class LedgerRecoveryOp implements ReadCallback, AddCallback {
     static final Logger LOG = LoggerFactory.getLogger(LedgerRecoveryOp.class);
     LedgerHandle lh;
     int numResponsesPending;
@@ -61,70 +61,25 @@ class LedgerRecoveryOp implements ReadEn
     }
 
     public void initiate() {
-        /** 
+        ReadLastConfirmedOp rlcop = new ReadLastConfirmedOp(lh,
+                new ReadLastConfirmedOp.LastConfirmedDataCallback() {
+                public void readLastConfirmedDataComplete(int rc, RecoveryData data) {
+                    if (rc == BKException.Code.OK) {
+                        lh.lastAddPushed = lh.lastAddConfirmed = data.lastAddConfirmed;
+                        lh.length = data.length;
+                        doRecoveryRead();
+                    } else {
+                        cb.operationComplete(BKException.Code.ReadException, null);
+                    }
+                }
+                });
+
+        /**
          * Enable fencing on this op. When the read request reaches the bookies
          * server it will fence off the ledger, stopping any subsequent operation
          * from writing to it.
          */
-        int flags = BookieProtocol.FLAG_DO_FENCING;
-        for (int i = 0; i < lh.metadata.currentEnsemble.size(); i++) {
-            lh.bk.bookieClient.readEntry(lh.metadata.currentEnsemble.get(i), lh.ledgerId, 
-                                         BookieProtocol.LAST_ADD_CONFIRMED, this, i, flags);
-        }
-    }
-
-    public synchronized void readEntryComplete(final int rc, final long ledgerId, final long entryId,
-            final ChannelBuffer buffer, final Object ctx) {
-
-        // Already proceeding with recovery, nothing to do
-        if (proceedingWithRecovery) {
-            return;
-        }
-
-        int bookieIndex = (Integer) ctx;
-
-        numResponsesPending--;
-
-        boolean heardValidResponse = false;
-
-        if (rc == BKException.Code.OK) {
-            try {
-                RecoveryData recoveryData = lh.macManager.verifyDigestAndReturnLastConfirmed(buffer);
-                maxAddConfirmed = Math.max(maxAddConfirmed, recoveryData.lastAddConfirmed);
-                maxAddPushed = Math.max(maxAddPushed, recoveryData.entryId);
-                heardValidResponse = true;
-            } catch (BKDigestMatchException e) {
-                // Too bad, this bookie didnt give us a valid answer, we
-                // still might be able to recover though so continue
-                LOG.error("Mac mismatch while reading last entry from bookie: "
-                          + lh.metadata.currentEnsemble.get(bookieIndex));
-            }
-        }
-
-        if (rc == BKException.Code.NoSuchLedgerExistsException || rc == BKException.Code.NoSuchEntryException) {
-            // this still counts as a valid response, e.g., if the
-            // client
-            // crashed without writing any entry
-            heardValidResponse = true;
-        }
-
-        // other return codes dont count as valid responses
-        if (heardValidResponse && lh.distributionSchedule.canProceedWithRecovery(bookieIndex)) {
-            proceedingWithRecovery = true;
-            lh.lastAddPushed = lh.lastAddConfirmed = maxAddConfirmed;
-            lh.length = maxLength;
-            doRecoveryRead();
-            return;
-        }
-
-        if (numResponsesPending == 0) {
-            // Have got all responses back but was still not enough to
-            // start
-            // recovery, just fail the operation
-            LOG.error("While recovering ledger: " + ledgerId + " did not hear success responses from all quorums");
-            cb.operationComplete(BKException.Code.LedgerRecoveryException, null);
-        }
-
+        rlcop.initiateWithFencing();
     }
 
     /**
@@ -163,10 +118,9 @@ class LedgerRecoveryOp implements ReadEn
                         cb.operationComplete(BKException.Code.ZKException, null);
                     } else {
                         cb.operationComplete(BKException.Code.OK, null);
-                        LOG.debug("After closing length is: " + lh.getLength()); 
+                        LOG.debug("After closing length is: " + lh.getLength());
                     }
-                } 
-                
+                }
                 }, null, BKException.Code.LedgerClosedException);
             return;
         }

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java Mon Feb 13 14:46:38 2012
@@ -33,29 +33,41 @@ import org.jboss.netty.buffer.ChannelBuf
 class ReadLastConfirmedOp implements ReadEntryCallback {
     static final Logger LOG = LoggerFactory.getLogger(LedgerRecoveryOp.class);
     LedgerHandle lh;
-    Object ctx;
     int numResponsesPending;
-    int validResponses;
-    long maxAddConfirmed;
-    long maxLength = 0;
+    RecoveryData maxRecoveredData;
     volatile boolean completed = false;
 
-    ReadLastConfirmedCallback cb;
+    LastConfirmedDataCallback cb;
+    final DistributionSchedule.QuorumCoverageSet coverageSet;
 
-    public ReadLastConfirmedOp(LedgerHandle lh, ReadLastConfirmedCallback cb, Object ctx) {
+    /**
+     * Wrapper to get all recovered data from the request
+     */
+    interface LastConfirmedDataCallback {
+        public void readLastConfirmedDataComplete(int rc, RecoveryData data);
+    }
+
+    public ReadLastConfirmedOp(LedgerHandle lh, LastConfirmedDataCallback cb) {
         this.cb = cb;
-        this.ctx = ctx;
+        this.maxRecoveredData = new RecoveryData(-1,-1,0);
         this.lh = lh;
-        this.maxAddConfirmed = -1L;
-        this.validResponses = 0;
         this.numResponsesPending = lh.metadata.ensembleSize;
+        this.coverageSet = lh.distributionSchedule.getCoverageSet();
     }
 
     public void initiate() {
-        LOG.info("### Initiate ###");
+        initiate(BookieProtocol.FLAG_NONE);
+    }
+
+    public void initiateWithFencing() {
+        initiate(BookieProtocol.FLAG_DO_FENCING);
+    }
+
+    private void initiate(short flags) {
         for (int i = 0; i < lh.metadata.currentEnsemble.size(); i++) {
-            lh.bk.bookieClient.readEntry(lh.metadata.currentEnsemble.get(i), lh.ledgerId, BookieProtocol.LAST_ADD_CONFIRMED, 
-                                         this, i, BookieProtocol.FLAG_NONE);
+            lh.bk.bookieClient.readEntry(lh.metadata.currentEnsemble.get(i), lh.ledgerId,
+                                         BookieProtocol.LAST_ADD_CONFIRMED,
+                                         this, i, flags);
         }
     }
 
@@ -64,12 +76,14 @@ class ReadLastConfirmedOp implements Rea
         int bookieIndex = (Integer) ctx;
 
         numResponsesPending--;
-
+        boolean heardValidResponse = false;
         if (rc == BKException.Code.OK) {
             try {
                 RecoveryData recoveryData = lh.macManager.verifyDigestAndReturnLastConfirmed(buffer);
-                maxAddConfirmed = Math.max(maxAddConfirmed, recoveryData.lastAddConfirmed);
-                validResponses++;
+                if (recoveryData.lastAddConfirmed > maxRecoveredData.lastAddConfirmed) {
+                    maxRecoveredData = recoveryData;
+                }
+                heardValidResponse = true;
             } catch (BKDigestMatchException e) {
                 // Too bad, this bookie didn't give us a valid answer, we
                 // still might be able to recover though so continue
@@ -80,26 +94,26 @@ class ReadLastConfirmedOp implements Rea
 
         if (rc == BKException.Code.NoSuchLedgerExistsException || rc == BKException.Code.NoSuchEntryException) {
             // this still counts as a valid response, e.g., if the client crashed without writing any entry
-            validResponses++;
+            heardValidResponse = true;
         }
 
-        // other return codes don't count as valid responses
-        if ((validResponses >= lh.metadata.quorumSize) &&
-                !completed) {
+        // other return codes dont count as valid responses
+        if (heardValidResponse
+            && coverageSet.addBookieAndCheckCovered(bookieIndex)
+            && !completed) {
             completed = true;
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Read Complete with enough validResponses");
             }
-            if(maxAddConfirmed > lh.lastAddConfirmed) lh.lastAddConfirmed = maxAddConfirmed;
-            cb.readLastConfirmedComplete(BKException.Code.OK, maxAddConfirmed, this.ctx);
+
+            cb.readLastConfirmedDataComplete(BKException.Code.OK, maxRecoveredData);
             return;
         }
 
         if (numResponsesPending == 0 && !completed) {
-            completed = true;
             // Have got all responses back but was still not enough, just fail the operation
             LOG.error("While readLastConfirmed ledger: " + ledgerId + " did not hear success responses from all quorums");
-            cb.readLastConfirmedComplete(BKException.Code.LedgerRecoveryException, maxAddConfirmed, this.ctx);
+            cb.readLastConfirmedDataComplete(BKException.Code.LedgerRecoveryException, maxRecoveredData);
         }
 
     }

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RoundRobinDistributionSchedule.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RoundRobinDistributionSchedule.java?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RoundRobinDistributionSchedule.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RoundRobinDistributionSchedule.java Mon Feb 13 14:46:38 2012
@@ -31,10 +31,6 @@ class RoundRobinDistributionSchedule imp
     int quorumSize;
     int ensembleSize;
 
-    // covered[i] is true if the quorum starting at bookie index i has been
-    // covered by a recovery reply
-    boolean[] covered = null;
-    int numQuorumsUncovered;
 
     public RoundRobinDistributionSchedule(int quorumSize, int ensembleSize) {
         this.quorumSize = quorumSize;
@@ -57,31 +53,38 @@ class RoundRobinDistributionSchedule imp
 
     }
 
-    public synchronized boolean canProceedWithRecovery(int bookieIndexHeardFrom) {
-        if (covered == null) {
+    private class RRQuorumCoverageSet implements QuorumCoverageSet {
+        // covered[i] is true if the quorum starting at bookie index i has been
+        // covered by a recovery reply
+        private boolean[] covered = null;
+        private int numQuorumsUncovered;
+
+        private RRQuorumCoverageSet() {
             covered = new boolean[ensembleSize];
             numQuorumsUncovered = ensembleSize;
         }
 
-        if (numQuorumsUncovered == 0) {
-            return true;
-        }
-
-        for (int i = 0; i < quorumSize; i++) {
-            int quorumStartIndex = MathUtils.signSafeMod(bookieIndexHeardFrom - i, ensembleSize);
-            if (!covered[quorumStartIndex]) {
-                covered[quorumStartIndex] = true;
-                numQuorumsUncovered--;
+        public synchronized boolean addBookieAndCheckCovered(int bookieIndexHeardFrom) {
+            if (numQuorumsUncovered == 0) {
+                return true;
+            }
 
-                if (numQuorumsUncovered == 0) {
-                    return true;
+            for (int i = 0; i < quorumSize; i++) {
+                int quorumStartIndex = MathUtils.signSafeMod(bookieIndexHeardFrom - i, ensembleSize);
+                if (!covered[quorumStartIndex]) {
+                    covered[quorumStartIndex] = true;
+                    numQuorumsUncovered--;
+
+                    if (numQuorumsUncovered == 0) {
+                        return true;
+                    }
                 }
             }
-
+            return false;
         }
-
-        return false;
-
     }
 
+    public QuorumCoverageSet getCoverageSet() {
+        return new RRQuorumCoverageSet();
+    }
 }

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=1243539&r1=1243538&r2=1243539&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 Feb 13 14:46:38 2012
@@ -405,7 +405,6 @@ public class PerChannelBookieClient exte
         LOG.info("Disconnected from bookie: " + addr);
         errorOutOutstandingEntries();
         channel.close();
-
         state = ConnectionState.DISCONNECTED;
 
         // we don't want to reconnect right away. If someone sends a request to

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java Mon Feb 13 14:46:38 2012
@@ -237,8 +237,7 @@ public class BookieRecoveryTest extends 
         bs.remove(0);
 
         // Startup a new bookie server
-        int newBookiePort = initialPort + numBookies;
-        startNewBookie(newBookiePort);
+        int newBookiePort = startNewBookie();
 
         // Write some more entries for the ledgers so a new ensemble will be
         // created for them.
@@ -292,8 +291,7 @@ public class BookieRecoveryTest extends 
 
         // Startup three new bookie servers
         for (int i = 0; i < 3; i++) {
-            int newBookiePort = initialPort + numBookies + i;
-            startNewBookie(newBookiePort);
+            startNewBookie();
         }
 
         // Write some more entries for the ledgers so a new ensemble will be
@@ -346,8 +344,7 @@ public class BookieRecoveryTest extends 
         bs.remove(0);
 
         // Startup a new bookie server
-        int newBookiePort = initialPort + numBookies;
-        startNewBookie(newBookiePort);
+        int newBookiePort = startNewBookie();
 
         // Write some more entries for the ledgers so a new ensemble will be
         // created for them.
@@ -391,8 +388,7 @@ public class BookieRecoveryTest extends 
 
         // Startup three new bookie servers
         for (int i = 0; i < 3; i++) {
-            int newBookiePort = initialPort + numBookies + i;
-            startNewBookie(newBookiePort);
+            startNewBookie();
         }
 
         // Write some more entries for the ledgers so a new ensemble will be
@@ -561,8 +557,7 @@ public class BookieRecoveryTest extends 
             bs.remove(removeIndex);
             
             // Startup three new bookie servers
-            int newBookiePort = initialPort + numBookies + i;
-            startNewBookie(newBookiePort);
+            startNewBookie();
             
             // Write some more entries for the ledgers so a new ensemble will be
             // created for them.

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java Mon Feb 13 14:46:38 2012
@@ -72,6 +72,7 @@ public abstract class BaseTestCase exten
     protected List<BookieServer> bs = new ArrayList<BookieServer>();
     protected List<ServerConfiguration> bsConfs = new ArrayList<ServerConfiguration>();
     protected Integer initialPort = 5000;
+    private Integer nextPort = initialPort;
     protected int numBookies;
     protected BookKeeperTestClient bkc;
 
@@ -142,7 +143,7 @@ public abstract class BaseTestCase exten
                 f.mkdir();
 
                 ServerConfiguration conf = newServerConfiguration(
-                    initialPort + i, HOSTPORT, f, new File[] { f });
+                    nextPort++, HOSTPORT, f, new File[] { f });
                 bsConfs.add(conf);
 
                 bs.add(startBookie(conf));
@@ -166,6 +167,15 @@ public abstract class BaseTestCase exten
         }
     }
 
+    public void killBookie(int index) throws InterruptedException, IOException {
+        if (index >= bs.size()) {
+            throw new IOException("Bookie does not exist");
+        }
+        BookieServer server = bs.get(index);
+        server.shutdown();
+        bs.remove(server);
+    }
+
     public void sleepBookie(InetSocketAddress addr, final int seconds,
                             final CountDownLatch l)
             throws InterruptedException, IOException {
@@ -235,16 +245,19 @@ public abstract class BaseTestCase exten
      *            Port to start the new bookie server on
      * @throws IOException
      */
-    protected void startNewBookie(int port)
+    protected int startNewBookie()
             throws IOException, InterruptedException, KeeperException, BookieException {
         File f = File.createTempFile("bookie", "test");
         tmpDirs.add(f);
         f.delete();
         f.mkdir();
 
+        int port = nextPort++;
         ServerConfiguration conf = newServerConfiguration(port, HOSTPORT, f, new File[] { f });
 
         bs.add(startBookie(conf));
+
+        return port;
     }
 
     /**
@@ -266,6 +279,7 @@ public abstract class BaseTestCase exten
 
         bkc.readBookiesBlocking();
         LOG.info("New bookie on port " + port + " has been created.");
+
         return server;
     }
 

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=1243539&r1=1243538&r2=1243539&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 Feb 13 14:46:38 2012
@@ -302,4 +302,80 @@ public class BookieFailureTest extends B
         }
     }
 
+    @Test
+    public void testLedgerNoRecoveryOpenAfterBKCrashed() throws Exception {
+        // Create a ledger
+        LedgerHandle beforelh = bkc.createLedger(numBookies, numBookies, 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);
+
+        // try to open ledger no recovery
+        LedgerHandle afterlh = bkc.openLedgerNoRecovery(beforelh.getId(), digestType, "".getBytes());
+
+        assertEquals(numEntries - 2, afterlh.getLastAddConfirmed());
+
+        startNewBookie();
+        LedgerHandle beforelh2 = bkc.createLedger(numBookies, 1, digestType, "".getBytes());
+
+        for (int i=0; i<numEntries; i++) {
+            beforelh2.addEntry(tmp.getBytes());
+        }
+
+        // shutdown first bookie server
+        killBookie(0);
+
+        // try to open ledger no recovery
+        try {
+            bkc.openLedgerNoRecovery(beforelh2.getId(), digestType, "".getBytes());
+            fail("Should have thrown exception");
+        } catch (BKException.BKReadException e) {
+            // correct behaviour
+        }
+    }
+
+    @Test
+    public void testLedgerOpenAfterBKCrashed() throws Exception {
+        // Create a ledger
+        LedgerHandle beforelh = bkc.createLedger(numBookies, numBookies, 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();
+
+        // try to open ledger no recovery
+        LedgerHandle afterlh = bkc.openLedger(beforelh.getId(), digestType, "".getBytes());
+
+        assertEquals(beforelh.getLastAddPushed(), afterlh.getLastAddConfirmed());
+
+        LedgerHandle beforelh2 = bkc.createLedger(numBookies, 1, digestType, "".getBytes());
+
+        for (int i=0; i<numEntries; i++) {
+            beforelh2.addEntry(tmp.getBytes());
+        }
+
+        // shutdown first bookie server
+        killBookie(0);
+
+        // try to open ledger no recovery
+        try {
+            bkc.openLedger(beforelh2.getId(), digestType, "".getBytes());
+            fail("Should have thrown exception");
+        } catch (BKException.BKLedgerRecoveryException e) {
+            // correct behaviour
+        }
+    }
+
 }

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerRecoveryTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerRecoveryTest.java?rev=1243539&r1=1243538&r2=1243539&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerRecoveryTest.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerRecoveryTest.java Mon Feb 13 14:46:38 2012
@@ -139,8 +139,7 @@ public class LedgerRecoveryTest extends 
         }
 
         // start a new bookie server
-        int newBookiePort = initialPort + numBookies;
-        startNewBookie(newBookiePort);
+        startNewBookie();
 
         LedgerHandle afterlh = bkc.openLedger(beforelh.getId(), digestType, "".getBytes());