----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8141/#review13823 -----------------------------------------------------------
a few more comments. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java <https://reviews.apache.org/r/8141/#comment29517> what are the case(s) in which end would be -1? bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java <https://reviews.apache.org/r/8141/#comment29519> all the cases I've seen in this patch use ANY version, so I was wondering about the motivation for the version input parameter and for deleting the znode. bookkeeper-server/src/main/java/org/apache/bookkeeper/util/StringUtils.java <https://reviews.apache.org/r/8141/#comment29520> could you move the "throws" part to the next line? - fpj On Nov. 21, 2012, 3:19 a.m., Fangmin Lv wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8141/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2012, 3:19 a.m.) > > > Review request for bookkeeper. > > > Description > ------- > > Main changes: > > 1. Refactor Garbage Collector Interface > 2. Change LedgerManager#deleteLedger to versioned delete > 3. Remove ActiveLedgerManager interface > 4. Move some common functions to StringUtils and ZkUtils > > > This addresses bug BOOKKEEPER-463. > https://issues.apache.org/jira/browse/BOOKKEEPER-463 > > > Diffs > ----- > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java > 929be51 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollector.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java > cecb74a > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java > c3f5149 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java > c8d2b21 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerDeleteOp.java > eae1f37 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java > 9dcb1b9 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ActiveLedgerManager.java > 542b498 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java > e284776 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManagerFactory.java > 329e0a7 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java > 3499a05 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManagerFactory.java > c86b884 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java > 30e2b83 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManagerFactory.java > a7fc247 > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/SnapshotMap.java > c222f05 > bookkeeper-server/src/main/java/org/apache/bookkeeper/util/StringUtils.java > 575e480 > bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java > 4073450 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java > a24b1e2 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java > 7ecf937 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerTestCase.java > cd0b91f > > Diff: https://reviews.apache.org/r/8141/diff/ > > > Testing > ------- > > > Thanks, > > Fangmin Lv > >