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 2013/09/10 15:13:52 UTC
svn commit: r1521471 - in /zookeeper/bookkeeper/branches/branch-4.2: ./
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/
bookkeeper-server/src/main/java/org/apache/bookkeeper/repl...
Author: ivank
Date: Tue Sep 10 13:13:52 2013
New Revision: 1521471
URL: http://svn.apache.org/r1521471
Log:
BOOKKEEPER-446: BookKeeper.createLedger(..) should not mask the error with ZKException (sijie via ivank)
Modified:
zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.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=1521471&r1=1521470&r2=1521471&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt Tue Sep 10 13:13:52 2013
@@ -76,6 +76,8 @@ Release 4.2.2 - Unreleased
BOOKKEEPER-669: Race condition in ledger deletion and eviction from cache (rakeshr via ivank)
+ BOOKKEEPER-446: BookKeeper.createLedger(..) should not mask the error with ZKException (sijie via ivank)
+
hedwig-server:
BOOKKEEPER-579: TestSubAfterCloseSub was put in a wrong package (sijie via ivank)
Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java?rev=1521471&r1=1521470&r2=1521471&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java Tue Sep 10 13:13:52 2013
@@ -88,8 +88,12 @@ public abstract class BKException extend
return new BKUnclosedFragmentException();
case Code.WriteOnReadOnlyBookieException:
return new BKWriteOnReadOnlyBookieException();
- default:
+ case Code.ReplicationException:
+ return new BKReplicationException();
+ case Code.IllegalOpException:
return new BKIllegalOpException();
+ default:
+ return new BKUnexpectedConditionException();
}
}
@@ -123,6 +127,12 @@ public abstract class BKException extend
int UnauthorizedAccessException = -102;
int UnclosedFragmentException = -103;
int WriteOnReadOnlyBookieException = -104;
+
+ // generic exception code used to propagate in replication pipeline
+ int ReplicationException = -200;
+
+ // For all unexpected error conditions
+ int UnexpectedConditionException = -999;
}
public void setCode(int code) {
@@ -181,8 +191,12 @@ public abstract class BKException extend
return "Attempting to use an unclosed fragment; This is not safe";
case Code.WriteOnReadOnlyBookieException:
return "Attempting to write on ReadOnly bookie";
- default:
+ case Code.ReplicationException:
+ return "Errors in replication pipeline";
+ case Code.IllegalOpException:
return "Invalid operation";
+ default:
+ return "Unexpected condition";
}
}
@@ -228,6 +242,12 @@ public abstract class BKException extend
}
}
+ public static class BKUnexpectedConditionException extends BKException {
+ public BKUnexpectedConditionException() {
+ super(Code.UnexpectedConditionException);
+ }
+ }
+
public static class BKNotEnoughBookiesException extends BKException {
public BKNotEnoughBookiesException() {
super(Code.NotEnoughBookiesException);
@@ -323,4 +343,10 @@ public abstract class BKException extend
super(Code.WriteOnReadOnlyBookieException);
}
}
+
+ public static class BKReplicationException extends BKException {
+ public BKReplicationException() {
+ super(Code.ReplicationException);
+ }
+ }
}
Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java?rev=1521471&r1=1521470&r2=1521471&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java Tue Sep 10 13:13:52 2013
@@ -377,9 +377,12 @@ public class BookKeeper {
* Wait
*/
counter.block(0);
- if (counter.getLh() == null) {
- LOG.error("ZooKeeper error: " + counter.getrc());
- throw BKException.create(Code.ZKException);
+ if (counter.getrc() != BKException.Code.OK) {
+ LOG.error("Error while creating ledger : {}", counter.getrc());
+ throw BKException.create(counter.getrc());
+ } else if (counter.getLh() == null) {
+ LOG.error("Unexpected condition : no ledger handle returned for a success ledger creation");
+ throw BKException.create(BKException.Code.UnexpectedConditionException);
}
return counter.getLh();
Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java?rev=1521471&r1=1521470&r2=1521471&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java Tue Sep 10 13:13:52 2013
@@ -132,9 +132,9 @@ class LedgerRecoveryOp implements ReadCa
lh.asyncCloseInternal(new CloseCallback() {
@Override
public void closeComplete(int rc, LedgerHandle lh, Object ctx) {
- if (rc != KeeperException.Code.OK.intValue()) {
+ if (rc != BKException.Code.OK) {
LOG.warn("Close failed: " + BKException.getMessage(rc));
- cb.operationComplete(BKException.Code.ZKException, null);
+ cb.operationComplete(rc, null);
} else {
cb.operationComplete(BKException.Code.OK, null);
LOG.debug("After closing length is: {}", lh.getLength());
Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java?rev=1521471&r1=1521470&r2=1521471&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java Tue Sep 10 13:13:52 2013
@@ -79,13 +79,13 @@ class FlatLedgerManager extends Abstract
if (Code.OK.intValue() != rc) {
LOG.error("Could not create node for ledger",
KeeperException.create(KeeperException.Code.get(rc), path));
- cb.operationComplete(rc, null);
+ cb.operationComplete(BKException.Code.ZKException, null);
} else {
// update znode status
metadata.setVersion(new ZkVersion(0));
try {
long ledgerId = getLedgerId(name);
- cb.operationComplete(rc, ledgerId);
+ cb.operationComplete(BKException.Code.OK, ledgerId);
} catch (IOException ie) {
LOG.error("Could not extract ledger-id from path:" + name, ie);
cb.operationComplete(BKException.Code.ZKException, null);
Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java?rev=1521471&r1=1521470&r2=1521471&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java Tue Sep 10 13:13:52 2013
@@ -28,6 +28,7 @@ import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.bookkeeper.client.BKException;
import org.apache.bookkeeper.client.LedgerMetadata;
import org.apache.bookkeeper.conf.AbstractConfiguration;
import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback;
@@ -109,7 +110,7 @@ class HierarchicalLedgerManager extends
if (rc != KeeperException.Code.OK.intValue()) {
LOG.error("Could not generate new ledger id",
KeeperException.create(KeeperException.Code.get(rc), path));
- ledgerCb.operationComplete(rc, null);
+ ledgerCb.operationComplete(BKException.Code.ZKException, null);
return;
}
/*
@@ -120,7 +121,7 @@ class HierarchicalLedgerManager extends
ledgerId = getLedgerIdFromGenPath(idPathName);
} catch (IOException e) {
LOG.error("Could not extract ledger-id from id gen path:" + path, e);
- ledgerCb.operationComplete(KeeperException.Code.SYSTEMERROR.intValue(), null);
+ ledgerCb.operationComplete(BKException.Code.ZKException, null);
return;
}
String ledgerPath = getLedgerPath(ledgerId);
@@ -132,11 +133,11 @@ class HierarchicalLedgerManager extends
if (rc != KeeperException.Code.OK.intValue()) {
LOG.error("Could not create node for ledger",
KeeperException.create(KeeperException.Code.get(rc), path));
- ledgerCb.operationComplete(rc, null);
+ ledgerCb.operationComplete(BKException.Code.ZKException, null);
} else {
// update version
metadata.setVersion(new ZkVersion(0));
- ledgerCb.operationComplete(rc, lid);
+ ledgerCb.operationComplete(BKException.Code.OK, lid);
}
}
};
Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java?rev=1521471&r1=1521470&r2=1521471&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java Tue Sep 10 13:13:52 2013
@@ -64,10 +64,8 @@ import org.apache.commons.collections.Co
import com.google.common.collect.Sets;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.WatchedEvent;
-import org.apache.zookeeper.Watcher;
import org.apache.zookeeper.ZooKeeper;
import org.apache.zookeeper.AsyncCallback;
-import org.apache.zookeeper.Watcher.Event.KeeperState;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -178,8 +176,6 @@ public class Auditor implements BookiesL
}
} catch (BKException bke) {
LOG.error("Exception getting bookie list", bke);
- } catch (KeeperException ke) {
- LOG.error("Exception while watching available bookies", ke);
} catch (InterruptedException ie) {
Thread.currentThread().interrupt();
LOG.error("Interrupted while watching available bookies ", ie);
@@ -306,8 +302,7 @@ public class Auditor implements BookiesL
}
private void handleLostBookies(Collection<String> lostBookies,
- Map<String, Set<Long>> ledgerDetails) throws BKAuditException,
- KeeperException, InterruptedException {
+ Map<String, Set<Long>> ledgerDetails) throws BKAuditException {
LOG.info("Following are the failed bookies: " + lostBookies
+ " and searching its ledgers for re-replication");
@@ -319,7 +314,7 @@ public class Auditor implements BookiesL
}
private void publishSuspectedLedgers(String bookieIP, Set<Long> ledgers)
- throws KeeperException, InterruptedException, BKAuditException {
+ throws BKAuditException {
if (null == ledgers || ledgers.size() == 0) {
// there is no ledgers available for this bookie and just
// ignoring the bookie failures
@@ -368,12 +363,7 @@ public class Auditor implements BookiesL
} catch (BKException bke) {
LOG.error("Error closing lh", bke);
if (rc == BKException.Code.OK) {
- rc = BKException.Code.ZKException;
- }
- } catch (KeeperException ke) {
- LOG.error("Couldn't publish suspected ledger", ke);
- if (rc == BKException.Code.OK) {
- rc = BKException.Code.ZKException;
+ rc = BKException.Code.ReplicationException;
}
} catch (InterruptedException ie) {
LOG.error("Interrupted publishing suspected ledger", ie);
@@ -384,7 +374,7 @@ public class Auditor implements BookiesL
} catch (BKAuditException bkae) {
LOG.error("Auditor exception publishing suspected ledger", bkae);
if (rc == BKException.Code.OK) {
- rc = BKException.Code.ZKException;
+ rc = BKException.Code.ReplicationException;
}
}
Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.java?rev=1521471&r1=1521470&r2=1521471&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.java Tue Sep 10 13:13:52 2013
@@ -21,9 +21,12 @@ package org.apache.bookkeeper.test;
*
*/
+import static org.junit.Assert.fail;
+
import java.util.ArrayList;
import org.apache.bookkeeper.client.LedgerHandle;
+import org.apache.bookkeeper.client.BKException;
import org.apache.bookkeeper.client.BookKeeper.DigestType;
import org.junit.Before;
@@ -71,4 +74,26 @@ public class LedgerCreateDeleteTest exte
lh.close();
}
}
+
+ @Test(timeout = 60000)
+ public void testCreateLedgerWithBKNotEnoughBookiesException() throws Exception {
+ try {
+ bkc.createLedger(2, 2, DigestType.CRC32, "bk is cool".getBytes());
+ fail("Should be able to throw BKNotEnoughBookiesException");
+ } catch (BKException.BKNotEnoughBookiesException bkn) {
+ // expected
+ }
+ }
+
+ @Test(timeout = 60000)
+ public void testCreateLedgerWithZKException() throws Exception {
+ stopZKCluster();
+ try {
+ bkc.createLedger(1, 1, DigestType.CRC32, "bk is cool".getBytes());
+ fail("Should be able to throw ZKException");
+ } catch (BKException.ZKException zke) {
+ // expected
+ }
+ }
+
}