----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33096/#review84268 -----------------------------------------------------------
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java <https://reviews.apache.org/r/33096/#comment135453> why not create the sorted ledger storage also by reflection? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java <https://reviews.apache.org/r/33096/#comment135451> could you keep the scope as before? if you want to introduce a new scope 'storage', you could scope it in the new storage implementation. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java <https://reviews.apache.org/r/33096/#comment135456> could you move the CompactableLedgerStorage out of GarbageCollectionThread to a separated file in bookie package? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java <https://reviews.apache.org/r/33096/#comment135454> trailing space? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java <https://reviews.apache.org/r/33096/#comment135458> the comment isn't accurate. it isn't syncing the underlying storage though. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java <https://reviews.apache.org/r/33096/#comment135455> trailing space? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java <https://reviews.apache.org/r/33096/#comment135457> moved this to a separated file under bookie package. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java <https://reviews.apache.org/r/33096/#comment135459> this seems incorrect. you'd better to have a separate method 'syncEntriesLocations', which differentiate 'updateEntriesLocations' from 'syncEntriesLocations'. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java <https://reviews.apache.org/r/33096/#comment135463> this changes the behavior here. it doesn't need to load file info to file info cache before. but right now, it would, which it would pollute the file info cache and evict other active file infos. you might consider adding a separate interface for CompatableLedgerStorage. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java <https://reviews.apache.org/r/33096/#comment135461> @Override bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java <https://reviews.apache.org/r/33096/#comment135466> why change this logic here? It'd better to not to change it if it isn't a problem. bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java <https://reviews.apache.org/r/33096/#comment135464> trailing space bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java <https://reviews.apache.org/r/33096/#comment135465> trailing space - Sijie Guo On April 10, 2015, 10:18 p.m., Matteo Merli wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33096/ > ----------------------------------------------------------- > > (Updated April 10, 2015, 10:18 p.m.) > > > Review request for bookkeeper. > > > Bugs: BOOKKEEPER-851 > https://issues.apache.org/jira/browse/BOOKKEEPER-851 > > > Repository: bookkeeper-git > > > Description > ------- > > Allow to configure a different implementation for the LedgerStorage interface > in the Bookie configuration. > > > Diffs > ----- > > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java > 9036182 > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java > 8b40853 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java > 299fb3e > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java > 1a69692 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerStorage.java > e992d03 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerStorageFactory.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ReadOnlyEntryLogger.java > a2ce9e3 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java > d8a87e4 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java > 0a3884c > > bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java > cd9f7a0 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java > 6c9c4a7 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestSyncThread.java > d947dff > > bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java > 19aab44 > > Diff: https://reviews.apache.org/r/33096/diff/ > > > Testing > ------- > > > Thanks, > > Matteo Merli > >
