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 A7C599F22 for ; Tue, 17 Jan 2012 15:52:40 +0000 (UTC) Received: (qmail 29599 invoked by uid 500); 17 Jan 2012 15:52:40 -0000 Delivered-To: apmail-zookeeper-commits-archive@zookeeper.apache.org Received: (qmail 29457 invoked by uid 500); 17 Jan 2012 15:52:39 -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 29440 invoked by uid 99); 17 Jan 2012 15:52:39 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 17 Jan 2012 15:52:39 +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; Tue, 17 Jan 2012 15:52:37 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id A019B2388860 for ; Tue, 17 Jan 2012 15:52:17 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1232448 - in /zookeeper/bookkeeper/trunk: ./ bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ Date: Tue, 17 Jan 2012 15:52:17 -0000 To: commits@zookeeper.apache.org From: ivank@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20120117155217.A019B2388860@eris.apache.org> Author: ivank Date: Tue Jan 17 15:52:16 2012 New Revision: 1232448 URL: http://svn.apache.org/viewvc?rev=1232448&view=rev Log: BOOKKEEPER-150: Entry is lost when recovering a ledger with not enough bookies. (Sijie Guo via ivank) Modified: zookeeper/bookkeeper/trunk/CHANGES.txt 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/LedgerMetadata.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/LedgerRecoveryTest.java Modified: zookeeper/bookkeeper/trunk/CHANGES.txt URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1232448&r1=1232447&r2=1232448&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/CHANGES.txt (original) +++ zookeeper/bookkeeper/trunk/CHANGES.txt Tue Jan 17 15:52:16 2012 @@ -20,6 +20,8 @@ Trunk (unreleased changes) BOOKKEEPER-40: BookieClientTest fails intermittantly (fpj via ivank) + BOOKKEEPER-150: Entry is lost when recovering a ledger with not enough bookies. (Sijie Guo 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/client/LedgerHandle.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java?rev=1232448&r1=1232447&r2=1232448&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 Tue Jan 17 15:52:16 2012 @@ -525,6 +525,12 @@ public class LedgerHandle { // close the ledger and send fails to all the adds in the pipeline void handleUnrecoverableErrorDuringAdd(int rc) { + if (metadata.isInRecovery()) { + // we should not close ledger if ledger is recovery mode + // otherwise we may lose entry. + errorOutPendingAdds(rc); + return; + } asyncCloseInternal(NoopCloseCallback.instance, null, rc); } Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java?rev=1232448&r1=1232447&r2=1232448&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java Tue Jan 17 15:52:16 2012 @@ -93,6 +93,10 @@ public class LedgerMetadata { return close != NOTCLOSED && close != IN_RECOVERY; } + + boolean isInRecovery() { + return IN_RECOVERY == close; + } void markLedgerInRecovery() { close = IN_RECOVERY; 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=1232448&r1=1232447&r2=1232448&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 Tue Jan 17 15:52:16 2012 @@ -180,35 +180,6 @@ public class BookieRecoveryTest extends } /** - * Helper method to startup a new bookie server with the indicated port - * number - * - * @param port - * Port to start the new bookie server on - * @throws IOException - */ - private void startNewBookie(int port) - throws IOException, InterruptedException, KeeperException { - File f = File.createTempFile("bookie", "test"); - tmpDirs.add(f); - f.delete(); - f.mkdir(); - - ServerConfiguration conf = newServerConfiguration(port, HOSTPORT, f, new File[] { f }); - - BookieServer server = new BookieServer(conf); - server.start(); - bs.add(server); - - while(bkc.getZkHandle().exists("/ledgers/available/" + InetAddress.getLocalHost().getHostAddress() + ":" + port, false) == null) { - Thread.sleep(500); - } - - bkc.readBookiesBlocking(); - LOG.info("New bookie on port " + port + " has been created."); - } - - /** * Helper method to verify that we can read the recovered ledger entries. * * @param oldLhs 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=1232448&r1=1232447&r2=1232448&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 Tue Jan 17 15:52:16 2012 @@ -23,6 +23,7 @@ package org.apache.bookkeeper.test; import java.io.IOException; import java.io.File; +import java.net.InetAddress; import java.net.InetSocketAddress; import java.util.ArrayList; import java.util.Arrays; @@ -201,6 +202,35 @@ public abstract class BaseTestCase exten } } + /** + * Helper method to startup a new bookie server with the indicated port + * number + * + * @param port + * Port to start the new bookie server on + * @throws IOException + */ + protected void startNewBookie(int port) + throws IOException, InterruptedException, KeeperException { + File f = File.createTempFile("bookie", "test"); + tmpDirs.add(f); + f.delete(); + f.mkdir(); + + ServerConfiguration conf = newServerConfiguration(port, HOSTPORT, f, new File[] { f }); + + BookieServer server = new BookieServer(conf); + server.start(); + bs.add(server); + + while(bkc.getZkHandle().exists("/ledgers/available/" + InetAddress.getLocalHost().getHostAddress() + ":" + port, false) == null) { + Thread.sleep(500); + } + + bkc.readBookiesBlocking(); + LOG.info("New bookie on port " + port + " has been created."); + } + @After @Override public void tearDown() throws Exception { 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=1232448&r1=1232447&r2=1232448&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 Tue Jan 17 15:52:16 2012 @@ -111,4 +111,43 @@ public class LedgerRecoveryTest extends } } + @Test + public void testLedgerRecoveryWithNotEnoughBookies() throws Exception { + int numEntries = 3; + + // Create a ledger + LedgerHandle beforelh = null; + beforelh = bkc.createLedger(3, 3, digestType, "".getBytes()); + + String tmp = "BookKeeper is cool!"; + for (int i = 0; i < numEntries; i++) { + beforelh.addEntry(tmp.getBytes()); + } + + // shutdown first bookie server + bs.get(0).shutdown(); + bs.remove(0); + + /* + * Try to open ledger. + */ + try { + bkc.openLedger(beforelh.getId(), digestType, "".getBytes()); + fail("should not reach here!"); + } catch (Exception e) { + // should thrown recovery exception + } + + // start a new bookie server + int newBookiePort = initialPort + numBookies; + startNewBookie(newBookiePort); + + LedgerHandle afterlh = bkc.openLedger(beforelh.getId(), digestType, "".getBytes()); + + /* + * Check if has recovered properly. + */ + assertEquals(numEntries - 1, afterlh.getLastAddConfirmed()); + } + }