Hey, In InterleavedLedgerStorage, since the initial version of it ( https://github.com/apache/bookkeeper/commit/4a94ce1d8184f5f38def015d80777a8113b96690 and https://github.com/apache/bookkeeper/commit/d175ada58dcaf78f0a70b0ebebf489255ae67b5f), addEntry and processEntry methods are synchronized. If it is synchronized then I dont get what is the point in having 'writeThreadPool' in BookieRequestProcessor, if anyhow they are going to be executed sequentially because of synchronized addEntry method in InterleavedLedgerStorage.
If we look at the implementation of addEntry and processEntry method, 'somethingWritten' can be made threadsafe by using AtomicBoolean, ledgerCache.updateLastAddConfirmed and entryLogger.addEntry methods are inherently threadsafe. I'm not sure about semantics of ledgerCache.putEntryOffset method here. I'm not confident enough to say if LedgerCacheImpl and IndexInMemPageMgr (and probably IndexPersistenceMgr) are thread-safe classes. As far as I understood, if ledgerCache.putEntryOffset is thread safe, then I dont see the need of synchronization for those methods. In any case, if they are not thread-safe can you please say why it is not thread-safe and how we can do more granular synchronization at LedgerCacheImpl level, so that we can remove the need of synchrnoization at InterleavedLedgerStorage level. I'm currently working on Multiple Entrylogs - https://issues.apache.org/jira/browse/BOOKKEEPER-1041. To reap the benefits of multipleentrylogs feature from performance perspective, this synchrnoization should be taken care or atleast bring it down to more granular synchronization (if possible). @Override synchronized public long addEntry(ByteBuffer entry) throws IOException { long ledgerId = entry.getLong(); long entryId = entry.getLong(); long lac = entry.getLong(); entry.rewind(); processEntry(ledgerId, entryId, entry); ledgerCache.updateLastAddConfirmed(ledgerId, lac); return entryId; } synchronized protected void processEntry(long ledgerId, long entryId, ByteBuffer entry, boolean rollLog) throws IOException { somethingWritten = true; long pos = entryLogger.addEntry(ledgerId, entry, rollLog); ledgerCache.putEntryOffset(ledgerId, entryId, pos); } Thanks, Charan
