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 {