reddycharan commented on issue #570: Multiple active entrylogs URL: https://github.com/apache/bookkeeper/issues/570#issuecomment-369537838 >Just a question, is some code based on this change running in some production system ? @jvrao No, not yet, we are in the process of pushing all of our internal changes to the community code and eventually use the community repo version, so that we can be close to the community. Since we are in this transition phase we kept hold of any big changes to our repo and instead work directly with the community repo. >Effectively it seems to me that this new kind of storage is vert different from InterleavedStorage. I remember we already discussed about this. Is it really a benefit to change InterleavedS and SortedS and not to start from scratch eventually creating a base class for common code? Ideally with the design approach i chose, there shouldn't be any changes to InterleavedLedgerStorage class and SortedLedgerStorage class. Even now the changes made to these two classes are minimal. These changes are required for the following reasons - 1) Removal of unnecessary synchronization in InterleavedLedgerStorage methods. As described in http://mail-archives.apache.org/mod_mbox/bookkeeper-dev/201707.mbox/%3CCAO2yDyZ946fp2S_qR2iL178hPiXgrnFGb%3DpvkyK4ReSYAtNLBw%40mail.gmail.com%3E 2) minor changes to checkpoint logic, since in entryLogPerLedger it should not checkpoint when a new entrylog is created but instead it should checkpoint for every "flushInterval" period and also during this time it should flush both rotatedLogs and current active logs. 3) couple of minor changes in SortedLedgerStorage regarding how entryLogger methods are called and creation of flushexecutor and passing it to EntryMemTable in the case of entryLogPerLedger. The crux of the implementation change is in EntryLogger class and for the reasons explained above abstraction is done in EntryLogger class. >I think a more confortable way to get this in is to split the patch into smaller changes. @reddycharan Ok, I've spent multiple weeks on this work item. If it is going to make things easier on reviewers I can consider splitting this PR into multiple PRs (hoping it wont take heavy toll on me and complicate things further). Probably I can split like following **sub-task1** - Removal of unnecessary synchronization in InterleavedLedgerStorage methods. As described in http://mail-archives.apache.org/mod_mbox/bookkeeper-dev/201707.mbox/%3CCAO2yDyZ946fp2S_qR2iL178hPiXgrnFGb%3DpvkyK4ReSYAtNLBw%40mail.gmail.com%3E **sub-task2** - moved the complete logic of flushIntervalInBytes from EntryLogger to BufferedChannel **sub-task3** - make changes to SyncThread, so that incase of entrylogperledger, checkpoint happens for every flushInterval but not when active entrylog is created/rolled over. **sub-task4** - introduce parallel (have just one Runnable per ledger) EntryMemTable flusher for SortedLedgerStorage which can be used in the case of entrylogperledger **sub-task5** - Introduce EntryLogManager abstraction in EntryLogger and let it deal with the singleentrylog/entrylogperledger functionality and core business logic. As you can imagine, sub-task5 is going to be the crux of this work item and majority of my code change and the design choice of abstraction at EntryLogger is going to remain.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
