reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager. URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r176549632
########## File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java ########## @@ -802,88 +864,207 @@ private long readLastLogId(File f) { } } + interface EntryLogManager { + /* + * acquire lock for this ledger. + */ + void acquireLock(Long ledgerId); Review comment: currently in all the places 'synchronized' is used, but for entrylogperledger we should have a lock for each ledger/entrylog. Hence these lock methods in EntryLogManager. The contract with this interface is only in addEntry call "acquireLockByCreatingIfRequired" (because this call would create a lock if it is not created yet and we need this in addEntry method since addEntry will be the first call related to that ledger in write path) and for all other methods in EntryLogger when they are dealing/writing to that particular ledger/entrylog they would call "acquireLock". I don't see why it needs to be hidden. EntryLogger needs to synchronize, instead of synchronizing on its intrinsic lock it delegates it to EntryLogManager, and EntryLogManager deals with inner details but all it needs is call to acquireLock and releaseLock with ledgerId. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services