ivankelly commented on issue #570: Multiple active entrylogs URL: https://github.com/apache/bookkeeper/issues/570#issuecomment-368837292 > What I am suggesting here is to respect to what have been discussed and agreed on. Where has this been discussed? I can't find anything on the dev@ list. > His change is at the entry log manager level, which he attempts to provide a entry log manager to abstract how different implementations manage entry log files. what you suggest might be similar but slight different abstraction interface, which from my personal view, there is no real fundamental differences. There's a big difference. Making the change in the entrylogger itself, means that the implementation gets pulled into SortedLedgerStorage, and that means he has to deal with the EntryMemTable, which should have exactly nothing to do with the implementation. > so in my view, "shipit" is much important for a lot of organizations than other things. Shipping is a function of many things, not just getting code into branch. > That is a real issue for any changes and for any person reviewing the change. I don't think things are going to change if changing a different approach to implement this. You will probably feel it easy to review, because the implementation is aligned to what you have been thinking in your mind. 3000 LOC is never easy to review. 300-400 LOC is the max that can be reviewed competently in an hour, any more, people start skimming. I wrote an email about this in october[1], so I won't go through it again. Any way this is implemented, it needs to be broken into smaller patches (ideally 2-300LOC), so that each coherent piece can be thoroughly reviewed, and the reviewer can be sure that what is submitted is getting sufficient test coverage. If the patch cannot be broken, this in itself raises questions about the architecture of it. [1] https://lists.apache.org/thread.html/48db536c7208dcad87451d84e61959b0266cbe3860fcb0dafc451995@%3Cdev.bookkeeper.apache.org%3E
---------------------------------------------------------------- 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
