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/05/10 14:47:38 UTC

svn commit: r1336645 - in /zookeeper/bookkeeper/trunk: ./ bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ bookkeeper...

Author: ivank
Date: Thu May 10 12:47:37 2012
New Revision: 1336645

URL: http://svn.apache.org/viewvc?rev=1336645&view=rev
Log:
BOOKKEEPER-224: Fix findbugs in bookkeeper-server component (ivank)

Modified:
    zookeeper/bookkeeper/trunk/CHANGES.txt
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerEntryPage.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
    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/LedgerCreateOp.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/LedgerMetadata.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/meta/AbstractZkLedgerManager.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerLayout.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManagerFactory.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/HardLink.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/LocalBookKeeper.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/NIOServerFactoryTest.java

Modified: zookeeper/bookkeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/CHANGES.txt (original)
+++ zookeeper/bookkeeper/trunk/CHANGES.txt Thu May 10 12:47:37 2012
@@ -102,6 +102,8 @@ Trunk (unreleased changes)
 
 	BOOKKEEPER-235: Bad syncing in entrylogger degrades performance for many concurrent ledgers (ivank via fpj)
 
+        BOOKKEEPER-224: Fix findbugs in bookkeeper-server component (ivank)
+
       hedwig-client/
 
         BOOKKEEPER-217: NPE in hedwig client when enable DEBUG (sijie via ivank)

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java Thu May 10 12:47:37 2012
@@ -123,7 +123,7 @@ public class Bookie extends Thread {
     }
 
     // Write Callback do nothing
-    class NopWriteCallback implements WriteCallback {
+    static class NopWriteCallback implements WriteCallback {
         @Override
         public void writeComplete(int rc, long ledgerId, long entryId,
                                   InetSocketAddress addr, Object ctx) {
@@ -246,7 +246,11 @@ public class Bookie extends Thread {
                 LOG.error(err);
                 throw new IOException(err);
             }
-            dir.mkdirs();
+            if (!dir.mkdirs()) {
+                String err = "Unable to create directory " + dir;
+                LOG.error(err);
+                throw new IOException(err);
+            }
         }
     }
 

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java Thu May 10 12:47:37 2012
@@ -79,14 +79,19 @@ public abstract class BookieException ex
         switch(code) {
         case Code.OK:
             err = "No problem";
+            break;
         case Code.UnauthorizedAccessException:
             err = "Error while reading ledger";
+            break;
         case Code.LedgerFencedException:
             err = "Ledger has been fenced; No more entries can be added";
+            break;
         case Code.InvalidCookieException:
             err = "Invalid environment cookie found";
+            break;
         case Code.UpgradeException:
             err = "Error performing an upgrade operation ";
+            break;
         }
         String reason = super.getMessage();
         if (reason == null) {

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java Thu May 10 12:47:37 2012
@@ -57,7 +57,7 @@ public class EntryLogger {
     private static final Logger LOG = LoggerFactory.getLogger(EntryLogger.class);
     private File dirs[];
 
-    long logId;
+    private long logId;
     /**
      * The maximum size of a entry logger file.
      */
@@ -133,6 +133,10 @@ public class EntryLogger {
      */
     private ConcurrentHashMap<Long, BufferedChannel> channels = new ConcurrentHashMap<Long, BufferedChannel>();
 
+    synchronized long getCurrentLogId() {
+        return logId;
+    }
+
     /**
      * Creates a new log file
      */
@@ -190,7 +194,9 @@ public class EntryLogger {
                     + entryLogId + ".log");
             return false;
         }
-        entryLogFile.delete();
+        if (!entryLogFile.delete()) {
+            LOG.warn("Could not delete entry log file {}", entryLogFile);
+        }
         return true;
     }
 
@@ -206,7 +212,7 @@ public class EntryLogger {
             bw.flush();
         } finally {
             try {
-                fos.close();
+                bw.close();
             } catch (IOException e) {
             }
         }
@@ -263,7 +269,7 @@ public class EntryLogger {
             return -1;
         } finally {
             try {
-                fis.close();
+                br.close();
             } catch (IOException e) {
             }
         }
@@ -344,14 +350,13 @@ public class EntryLogger {
         // If the file already exists before creating a BufferedChannel layer above it,
         // set the FileChannel's position to the end so the write buffer knows where to start.
         newFc.position(newFc.size());
-        synchronized (channels) {
-            fc = channels.get(entryLogId);
-            if (fc != null) {
-                newFc.close();
-                return fc;
-            }
-            fc = new BufferedChannel(newFc, 8192);
-            channels.put(entryLogId, fc);
+        fc = new BufferedChannel(newFc, 8192);
+
+        BufferedChannel oldfc = channels.putIfAbsent(entryLogId, fc);
+        if (oldfc != null) {
+            newFc.close();
+            return oldfc;
+        } else {
             return fc;
         }
     }

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java Thu May 10 12:47:37 2012
@@ -163,7 +163,9 @@ public class FileSystemUpgrade {
 
         for (String f : files) {
             if (f.endsWith(".idx")) { // this is an index dir, create the links
-                targetPath.mkdirs();
+                if (!targetPath.mkdirs()) {
+                    throw new IOException("Could not create target path ["+targetPath+"]");
+                }
                 HardLink.createHardLinkMult(srcPath, files, targetPath);
                 return;
             }
@@ -198,7 +200,9 @@ public class FileSystemUpgrade {
                     File curDir = new File(d, Bookie.CURRENT_DIR);
                     File tmpDir = new File(d, "upgradeTmp." + System.nanoTime());
                     deferredMoves.put(curDir, tmpDir);
-                    tmpDir.mkdirs();
+                    if (!tmpDir.mkdirs()) {
+                        throw new BookieException.UpgradeException("Could not create temporary directory " + tmpDir);
+                    }
                     c.writeToDirectory(tmpDir);
 
                     String[] files = d.list(new FilenameFilter() {
@@ -251,14 +255,18 @@ public class FileSystemUpgrade {
                 if (version < 3) {
                     if (version == 2) {
                         File v2versionFile = new File(d, Cookie.VERSION_FILENAME);
-                        v2versionFile.delete();
+                        if (!v2versionFile.delete()) {
+                            LOG.warn("Could not delete old version file {}", v2versionFile);
+                        }
                     }
                     File[] files = d.listFiles(BOOKIE_FILES_FILTER);
                     for (File f : files) {
                         if (f.isDirectory()) {
                             FileUtils.deleteDirectory(f);
                         } else{
-                            f.delete();
+                            if (!f.delete()) {
+                                LOG.warn("Could not delete {}", f);
+                            }
                         }
                     }
                 }

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java Thu May 10 12:47:37 2012
@@ -457,7 +457,7 @@ public class GarbageCollectorThread exte
         // Extract it for every entry log except for the current one.
         // Entry Log ID's are just a long value that starts at 0 and increments
         // by 1 when the log fills up and we roll to a new one.
-        long curLogId = entryLogger.logId;
+        long curLogId = entryLogger.getCurrentLogId();
         for (long entryLogId = 0; entryLogId < curLogId; entryLogId++) {
             // Comb the current entry log file if it has not already been extracted.
             if (entryLogMetaMap.containsKey(entryLogId)) {

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java Thu May 10 12:47:37 2012
@@ -99,14 +99,25 @@ class Journal extends Thread {
         synchronized void markLog() {
             lastMark = new LastLogMark(txnLogId, txnLogPosition);
         }
+
+        synchronized LastLogMark getLastMark() {
+            return lastMark;
+        }
+        synchronized long getTxnLogId() {
+            return txnLogId;
+        }
+        synchronized long getTxnLogPosition() {
+            return txnLogPosition;
+        }
+
         synchronized void rollLog() {
             byte buff[] = new byte[16];
             ByteBuffer bb = ByteBuffer.wrap(buff);
             // we should record <logId, logPosition> marked in markLog
             // which is safe since records before lastMark have been
             // persisted to disk (both index & entry logger)
-            bb.putLong(lastMark.txnLogId);
-            bb.putLong(lastMark.txnLogPosition);
+            bb.putLong(lastMark.getTxnLogId());
+            bb.putLong(lastMark.getTxnLogPosition());
             if (LOG.isDebugEnabled()) {
                 LOG.debug("RollLog to persist last marked log : " + lastMark);
             }
@@ -135,8 +146,15 @@ class Journal extends Thread {
                 File file = new File(dir, "lastMark");
                 try {
                     FileInputStream fis = new FileInputStream(file);
-                    fis.read(buff);
-                    fis.close();
+                    try {
+                        int bytesRead = fis.read(buff);
+                        if (bytesRead != 16) {
+                            throw new IOException("Couldn't read enough bytes from lastMark."
+                                                  + " Wanted " + 16 + ", got " + bytesRead);
+                        }
+                    } finally {
+                        fis.close();
+                    }
                     bb.clear();
                     long i = bb.getLong();
                     long p = bb.getLong();
@@ -169,7 +187,7 @@ class Journal extends Thread {
     private class JournalRollingFilter implements JournalIdFilter {
         @Override
         public boolean accept(long journalId) {
-            if (journalId < lastLogMark.lastMark.txnLogId) {
+            if (journalId < lastLogMark.getLastMark().getTxnLogId()) {
                 return true;
             } else {
                 return false;
@@ -308,9 +326,11 @@ class Journal extends Thread {
             for (int i=0; i<maxIdx; i++) {
                 long id = logs.get(i);
                 // make sure the journal id is smaller than marked journal id
-                if (id < lastLogMark.lastMark.txnLogId) {
+                if (id < lastLogMark.getLastMark().getTxnLogId()) {
                     File journalFile = new File(journalDirectory, Long.toHexString(id) + ".txn");
-                    journalFile.delete();
+                    if (!journalFile.delete()) {
+                        LOG.warn("Could not delete old journal file {}", journalFile);
+                    }
                     LOG.info("garbage collected journal " + journalFile.getName());
                 }
             }
@@ -380,7 +400,7 @@ class Journal extends Thread {
      * @throws IOException
      */
     public void replay(JournalScanner scanner) throws IOException {
-        final long markedLogId = lastLogMark.txnLogId;
+        final long markedLogId = lastLogMark.getTxnLogId();
         List<Long> logs = listJournalIds(journalDirectory, new JournalIdFilter() {
             @Override
             public boolean accept(long journalId) {
@@ -406,7 +426,7 @@ class Journal extends Thread {
         for(Long id: logs) {
             long logPosition = 0L;
             if(id == markedLogId) {
-                logPosition = lastLogMark.txnLogPosition;
+                logPosition = lastLogMark.getTxnLogPosition();
             }
             scanJournal(id, logPosition, scanner);
         }
@@ -517,8 +537,10 @@ class Journal extends Thread {
             }
             logFile.close();
             logFile = null;
-        } catch (Exception e) {
-            LOG.warn("Journal exits when shutting down", e);
+        } catch (IOException ioe) {
+            LOG.error("I/O exception in Journal thread!", ioe);
+        } catch (InterruptedException ie) {
+            LOG.warn("Journal exits when shutting down", ie);
         } finally {
             IOUtils.close(LOG, logFile);
         }

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java Thu May 10 12:47:37 2012
@@ -440,7 +440,7 @@ public class LedgerCacheImpl implements 
             }
             totalWritten += rc;
         }
-        if (totalWritten != count * pageSize) {
+        if (totalWritten != (long)count * (long)pageSize) {
             throw new IOException("Short write to ledger " + ledger + " wrote " + totalWritten
                                   + " expected " + count * pageSize);
         }
@@ -727,12 +727,12 @@ public class LedgerCacheImpl implements 
 
             @Override
             public int getPageCount() {
-                return getNumUsedPages();
+                return LedgerCacheImpl.this.getNumUsedPages();
             }
 
             @Override
             public int getPageSize() {
-                return getPageSize();
+                return LedgerCacheImpl.this.getPageSize();
             }
 
             @Override
@@ -742,7 +742,7 @@ public class LedgerCacheImpl implements 
 
             @Override
             public int getPageLimit() {
-                return getPageLimit();
+                return LedgerCacheImpl.this.getPageLimit();
             }
 
             @Override

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerEntryPage.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerEntryPage.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerEntryPage.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerEntryPage.java Thu May 10 12:47:37 2012
@@ -80,8 +80,12 @@ public class LedgerEntryPage {
     }
     @Override
     public boolean equals(Object other) {
-        LedgerEntryPage otherLEP = (LedgerEntryPage) other;
-        return otherLEP.getLedger() == getLedger() && otherLEP.getFirstEntry() == getFirstEntry();
+        if (other instanceof LedgerEntryPage) {
+            LedgerEntryPage otherLEP = (LedgerEntryPage) other;
+            return otherLEP.getLedger() == getLedger() && otherLEP.getFirstEntry() == getFirstEntry();
+        } else {
+            return false;
+        }
     }
     @Override
     public int hashCode() {

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java Thu May 10 12:47:37 2012
@@ -183,7 +183,7 @@ public class BookKeeperAdmin {
     }
 
     // Object used for calling async methods and waiting for them to complete.
-    class SyncObject {
+    static class SyncObject {
         boolean value;
         int rc;
 
@@ -324,7 +324,7 @@ public class BookKeeperAdmin {
                         availableBookies.add(new InetSocketAddress(parts[0], Integer.parseInt(parts[1])));
                     }
                     // Now poll ZK to get the active ledgers
-                    getActiveLedgers(bookieSrc, bookieDest, cb, context, availableBookies);
+                    getActiveLedgers(bookieSrc, null, cb, context, availableBookies);
                 }
             }, null);
         }
@@ -702,7 +702,7 @@ public class BookKeeperAdmin {
      * Once finished propogate callback up to ledgerFragmentsMcb which should
      * be a multicallback responsible for all fragments in a single ledger
      */
-    class SingleFragmentCallback implements AsyncCallback.VoidCallback {
+    static class SingleFragmentCallback implements AsyncCallback.VoidCallback {
         final AsyncCallback.VoidCallback ledgerFragmentsMcb;
         final LedgerHandle lh;
         final long fragmentStartId;

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=1336645&r1=1336644&r2=1336645&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 Thu May 10 12:47:37 2012
@@ -163,12 +163,10 @@ abstract class DigestManager {
 
     static class RecoveryData {
         long lastAddConfirmed;
-        long entryId;
         long length;
 
-        public RecoveryData(long lastAddConfirmed, long entryId, long length) {
+        public RecoveryData(long lastAddConfirmed, long length) {
             this.lastAddConfirmed = lastAddConfirmed;
-            this.entryId = entryId;
             this.length = length;
         }
 
@@ -178,9 +176,9 @@ abstract class DigestManager {
         verifyDigest(dataReceived);
         dataReceived.readerIndex(8);
 
-        long entryId = dataReceived.readLong();
+        dataReceived.readLong(); // skip unused entryId
         long lastAddConfirmed = dataReceived.readLong();
         long length = dataReceived.readLong();
-        return new RecoveryData(lastAddConfirmed, entryId, length);
+        return new RecoveryData(lastAddConfirmed, length);
     }
 }

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java Thu May 10 12:47:37 2012
@@ -100,7 +100,7 @@ class LedgerCreateOp implements GenericC
         /*
          * Add ensemble to the configuration
          */
-        metadata.addEnsemble(new Long(0), ensemble);
+        metadata.addEnsemble(0L, ensemble);
 
         // create a ledger path with metadata
         bk.getLedgerManager().newLedgerPath(this, metadata);

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=1336645&r1=1336644&r2=1336645&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 Thu May 10 12:47:37 2012
@@ -25,6 +25,7 @@ import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.security.GeneralSecurityException;
 import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.Queue;
@@ -128,7 +129,7 @@ public class LedgerHandle {
      *
      * @return the id of the last entry pushed
      */
-    public long getLastAddPushed() {
+    synchronized public long getLastAddPushed() {
         return lastAddPushed;
     }
 
@@ -138,7 +139,7 @@ public class LedgerHandle {
      * @return byte array for the ledger's key/password.
      */
     public byte[] getLedgerKey() {
-        return ledgerKey;
+        return Arrays.copyOf(ledgerKey, ledgerKey.length);
     }
 
     /**
@@ -184,7 +185,7 @@ public class LedgerHandle {
      *
      * @return the length of the ledger in bytes
      */
-    public long getLength() {
+    synchronized public long getLength() {
         return this.length;
     }
 
@@ -399,7 +400,8 @@ public class LedgerHandle {
      */
     public void addEntry(byte[] data, int offset, int length)
             throws InterruptedException, BKException {
-        LOG.debug("Adding entry " + data);
+        LOG.debug("Adding entry {}", data);
+
         SyncCounter counter = new SyncCounter();
         counter.inc();
 
@@ -552,7 +554,7 @@ public class LedgerHandle {
     /**
      * Context objects for synchronous call to read last confirmed.
      */
-    class LastConfirmedCtx {
+    static class LastConfirmedCtx {
         long response;
         int rc;
 
@@ -754,7 +756,7 @@ public class LedgerHandle {
         }, null);
     }
 
-    void recover(final GenericCallback<Void> cb) {
+    synchronized void recover(final GenericCallback<Void> cb) {
         if (metadata.isClosed()) {
             lastAddConfirmed = lastAddPushed = metadata.close;
             length = metadata.length;

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=1336645&r1=1336644&r2=1336645&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 Thu May 10 12:47:37 2012
@@ -297,7 +297,7 @@ public class LedgerMetadata {
         for (int i=0; i<ensembles.size(); i++) {
             Long curKey = keyIter.next();
             Long newMetaKey = newMetaKeyIter.next();
-            if (curKey != newMetaKey) {
+            if (!curKey.equals(newMetaKey)) {
                 return false;
             }
         }

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=1336645&r1=1336644&r2=1336645&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 Thu May 10 12:47:37 2012
@@ -105,9 +105,10 @@ class LedgerRecoveryOp implements ReadCa
              * replicas. We subtract the length of the data itself, since it will
              * be added again when processing the call to add it.
              */
-            lh.length = entry.getLength() - (long) data.length;
+            synchronized (lh) {
+                lh.length = entry.getLength() - (long) data.length;
+            }
             lh.asyncRecoveryAddEntry(data, 0, data.length, this, null);
-
             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=1336645&r1=1336644&r2=1336645&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 Thu May 10 12:47:37 2012
@@ -49,7 +49,7 @@ class ReadLastConfirmedOp implements Rea
 
     public ReadLastConfirmedOp(LedgerHandle lh, LastConfirmedDataCallback cb) {
         this.cb = cb;
-        this.maxRecoveredData = new RecoveryData(-1,-1,0);
+        this.maxRecoveredData = new RecoveryData(-1,0);
         this.lh = lh;
         this.numResponsesPending = lh.metadata.ensembleSize;
         this.coverageSet = lh.distributionSchedule.getCoverageSet();

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java Thu May 10 12:47:37 2012
@@ -137,8 +137,9 @@ abstract class AbstractZkLedgerManager i
         }, null);
     }
 
-    private class GetLedgersCtx {
+    private static class GetLedgersCtx {
         int rc;
+        boolean done = false;
         HashSet<Long> ledgers = null;
     }
 
@@ -156,8 +157,7 @@ abstract class AbstractZkLedgerManager i
         if (LOG.isDebugEnabled()) {
             LOG.debug("Try to get ledgers of node : " + nodePath);
         }
-        synchronized (ctx) {
-            asyncGetLedgersInSingleNode(nodePath, new GenericCallback<HashSet<Long>>() {
+        asyncGetLedgersInSingleNode(nodePath, new GenericCallback<HashSet<Long>>() {
                 @Override
                 public void operationComplete(int rc, HashSet<Long> zkActiveLedgers) {
                     synchronized (ctx) {
@@ -165,11 +165,16 @@ abstract class AbstractZkLedgerManager i
                             ctx.ledgers = zkActiveLedgers;
                         }
                         ctx.rc = rc;
+                        ctx.done = true;
                         ctx.notifyAll();
                     }
                 }
             });
-            ctx.wait();
+
+        synchronized (ctx) {
+            while (ctx.done == false) {
+                ctx.wait();
+            }
         }
         if (Code.OK.intValue() != ctx.rc) {
             throw new IOException("Error on getting ledgers from node " + nodePath);

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerLayout.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerLayout.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerLayout.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerLayout.java Thu May 10 12:47:37 2012
@@ -194,6 +194,11 @@ class LedgerLayout {
     }
 
     @Override
+    public int hashCode() {
+        return (managerType + managerVersion).hashCode();
+    }
+
+    @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();
         sb.append("LV").append(layoutFormatVersion).append(":")

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManagerFactory.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManagerFactory.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManagerFactory.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManagerFactory.java Thu May 10 12:47:37 2012
@@ -50,7 +50,7 @@ public class LedgerManagerFactory {
         
         // if zk is null, return the default ledger manager
         if (zk == null) {
-            return new FlatLedgerManager(conf, zk, 
+            return new FlatLedgerManager(conf, null,
                     ledgerRootPath, FlatLedgerManager.CUR_VERSION);
         }
 

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java Thu May 10 12:47:37 2012
@@ -86,6 +86,7 @@ public class BookieServer implements NIO
         this.bookie.start();
 
         nioServerFactory = new NIOServerFactory(conf, this);
+        nioServerFactory.start();
         running = true;
         deathWatcher = new DeathWatcher(conf);
         deathWatcher.start();
@@ -493,7 +494,7 @@ public class BookieServer implements NIO
     /**
      * A cnxn wrapper for time
      */
-    class TimedCnxn {
+    static class TimedCnxn {
         Cnxn cnxn;
         long time;
 

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/NIOServerFactory.java Thu May 10 12:47:37 2012
@@ -83,7 +83,6 @@ public class NIOServerFactory extends Th
         ss.socket().bind(new InetSocketAddress(conf.getBookiePort()));
         ss.configureBlocking(false);
         ss.register(selector, SelectionKey.OP_ACCEPT);
-        start();
     }
 
     public InetSocketAddress getLocalAddress() {
@@ -188,10 +187,6 @@ public class NIOServerFactory extends Th
 
         int sessionTimeout;
 
-        int packetsSent;
-
-        int packetsReceived;
-
         void doIO(SelectionKey k) throws InterruptedException {
             try {
                 if (sock == null) {
@@ -490,16 +485,18 @@ public class NIOServerFactory extends Th
         }
 
         private class CnxnStats {
-            long packetsReceived;
+            int packetsSent = 0;
 
-            long packetsSent;
+            int packetsReceived = 0;
 
             /**
              * The number of requests that have been submitted but not yet
              * responded to.
              */
             public long getOutstandingRequests() {
-                return outstandingRequests;
+                synchronized(Cnxn.this) {
+                    return outstandingRequests;
+                }
             }
 
             public long getPacketsReceived() {

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=1336645&r1=1336644&r2=1336645&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 Thu May 10 12:47:37 2012
@@ -73,7 +73,7 @@ public class PerChannelBookieClient exte
     static final Logger LOG = LoggerFactory.getLogger(PerChannelBookieClient.class);
 
     static final long maxMemory = Runtime.getRuntime().maxMemory() / 5;
-    public static int MAX_FRAME_LENGTH = 2 * 1024 * 1024; // 2M
+    public static final int MAX_FRAME_LENGTH = 2 * 1024 * 1024; // 2M
 
     InetSocketAddress addr;
     Semaphore opCounterSem = new Semaphore(2000);
@@ -174,38 +174,29 @@ public class PerChannelBookieClient exte
     void connectIfNeededAndDoOp(GenericCallback<Void> op) {
         boolean doOpNow;
 
-        // common case without lock first
-        if (channel != null && state == ConnectionState.CONNECTED) {
-            doOpNow = true;
-        } else {
-
-            synchronized (this) {
-                // check 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
-                    pendingOps.add(op);
+        synchronized (this) {
+            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
+                pendingOps.add(op);
 
-                    connect();
-                }
+                connect();
             }
         }
 
         if (doOpNow) {
             op.operationComplete(BKException.Code.OK, null);
         }
-
     }
 
     /**
@@ -447,7 +438,9 @@ public class PerChannelBookieClient exte
         LOG.info("Disconnected from bookie: " + addr);
         errorOutOutstandingEntries();
         channel.close();
-        state = ConnectionState.DISCONNECTED;
+        synchronized (this) {
+            state = ConnectionState.DISCONNECTED;
+        }
 
         // we don't want to reconnect right away. If someone sends a request to
         // this address, we will reconnect.

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java Thu May 10 12:47:37 2012
@@ -64,7 +64,7 @@ public class BookKeeperTools {
         String zkServers = args[0];
         String bookieSrcString[] = args[1].split(":");
         if (bookieSrcString.length < 2) {
-            System.err.println("BookieSrc inputted has invalid name format (host:port expected): " + bookieSrcString);
+            System.err.println("BookieSrc inputted has invalid name format (host:port expected): " + args[1]);
             return;
         }
         final InetSocketAddress bookieSrc = new InetSocketAddress(bookieSrcString[0], Integer
@@ -74,7 +74,7 @@ public class BookKeeperTools {
             String bookieDestString[] = args[2].split(":");
             if (bookieDestString.length < 2) {
                 System.err.println("BookieDest inputted has invalid name format (host:port expected): "
-                                   + bookieDestString);
+                                   + args[2]);
                 return;
             }
             bookieDest = new InetSocketAddress(bookieDestString[0], Integer.parseInt(bookieDestString[1]));

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/HardLink.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/HardLink.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/HardLink.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/HardLink.java Thu May 10 12:47:37 2012
@@ -48,7 +48,7 @@ public class HardLink { 
     OS_TYPE_MAC
   }
   
-  public static OSType osType;
+  public static final OSType osType;
   private static HardLinkCommandGetter getHardLinkCommand;
   
   public final LinkStats linkStats; //not static

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/LocalBookKeeper.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/LocalBookKeeper.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/LocalBookKeeper.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/LocalBookKeeper.java Thu May 10 12:47:37 2012
@@ -86,8 +86,9 @@ public class LocalBookKeeper {
         //ServerStats.registerAsConcrete();
         //ClientBase.setupTestEnv();
         ZkTmpDir = File.createTempFile("zookeeper", "test");
-        ZkTmpDir.delete();
-        ZkTmpDir.mkdir();
+        if (!ZkTmpDir.delete() || !ZkTmpDir.mkdir()) {
+            throw new IOException("Couldn't create zk directory " + ZkTmpDir);
+        }
 
         try {
             zks = new ZooKeeperServer(ZkTmpDir, ZkTmpDir, ZooKeeperDefaultPort);
@@ -134,8 +135,9 @@ public class LocalBookKeeper {
 
         for(int i = 0; i < numberOfBookies; i++) {
             tmpDirs[i] = File.createTempFile("bookie" + Integer.toString(i), "test");
-            tmpDirs[i].delete();
-            tmpDirs[i].mkdir();
+            if (!tmpDirs[i].delete() || !tmpDirs[i].mkdir()) {
+                throw new IOException("Couldn't create bookie dir " + tmpDirs[i]);
+            }
 
             bsConfs[i] = new ServerConfiguration(baseConf);
             // override settings
@@ -183,7 +185,7 @@ public class LocalBookKeeper {
     }
 
     /*	User for testing purposes, void */
-    class emptyWatcher implements Watcher {
+    static class emptyWatcher implements Watcher {
         public void process(WatchedEvent event) {}
     }
 

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/NIOServerFactoryTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/NIOServerFactoryTest.java?rev=1336645&r1=1336644&r2=1336645&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/NIOServerFactoryTest.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/NIOServerFactoryTest.java Thu May 10 12:47:37 2012
@@ -49,6 +49,7 @@ public class NIOServerFactoryTest extend
         ServerConfiguration conf = new ServerConfiguration();
         conf.setBookiePort(22334);
         NIOServerFactory factory = new NIOServerFactory(conf, problemProcessor);
+        factory.start();
         Socket s = new Socket("127.0.0.1", 22334);
         s.setSoTimeout(5000);
         try {