zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From iv...@apache.org
Subject svn commit: r1521474 - in /zookeeper/bookkeeper/trunk: ./ 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/replication/ bookk...
Date Tue, 10 Sep 2013 13:14:41 GMT
Author: ivank
Date: Tue Sep 10 13:14:40 2013
New Revision: 1521474

URL: http://svn.apache.org/r1521474
Log:
BOOKKEEPER-446: BookKeeper.createLedger(..) should not mask the error with ZKException (sijie
via ivank)

Modified:
    zookeeper/bookkeeper/trunk/CHANGES.txt
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.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/meta/FlatLedgerManager.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.java

Modified: zookeeper/bookkeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1521474&r1=1521473&r2=1521474&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/CHANGES.txt (original)
+++ zookeeper/bookkeeper/trunk/CHANGES.txt Tue Sep 10 13:14:40 2013
@@ -98,6 +98,8 @@ Trunk (unreleased changes)
 
         BOOKKEEPER-580: improve close logic (sijie & ivank via ivank)
 
+        BOOKKEEPER-446: BookKeeper.createLedger(..) should not mask the error with ZKException
(sijie via ivank)
+
       hedwig-server:
 
         BOOKKEEPER-601: readahead cache size isn't updated correctly (sijie via fpj)

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java?rev=1521474&r1=1521473&r2=1521474&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java
Tue Sep 10 13:14:40 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/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java?rev=1521474&r1=1521473&r2=1521474&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
Tue Sep 10 13:14:40 2013
@@ -396,9 +396,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/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=1521474&r1=1521473&r2=1521474&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
Tue Sep 10 13:14:40 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/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java?rev=1521474&r1=1521473&r2=1521474&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
Tue Sep 10 13:14:40 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/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java?rev=1521474&r1=1521473&r2=1521474&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
Tue Sep 10 13:14:40 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/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java?rev=1521474&r1=1521473&r2=1521474&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
Tue Sep 10 13:14:40 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;
 
@@ -179,8 +177,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);
@@ -308,8 +304,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");
 
@@ -321,7 +316,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
@@ -370,12 +365,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);
@@ -386,7 +376,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/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.java?rev=1521474&r1=1521473&r2=1521474&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCreateDeleteTest.java
Tue Sep 10 13:14:40 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
+        }
+    }
+
 }



Mime
View raw message