> On Nov. 28, 2012, 6:10 p.m., fpj wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java, > > line 87 > > <https://reviews.apache.org/r/8141/diff/3/?file=222511#file222511line87> > > > > what are the case(s) in which end would be -1?
Sorry, this scenario will only occur in the the MSLedgerManager which haven't been in yet. > On Nov. 28, 2012, 6:10 p.m., fpj wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/util/StringUtils.java, > > line 100 > > <https://reviews.apache.org/r/8141/diff/3/?file=222522#file222522line100> > > > > could you move the "throws" part to the next line? Yes, I will ,thanks. > On Nov. 28, 2012, 6:10 p.m., fpj wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java, > > line 103 > > <https://reviews.apache.org/r/8141/diff/3/?file=222513#file222513line103> > > > > 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. We want a versioned remove, but there will be a lot of other changes related to versioned remove which is not about GC, maybe we can fire another jira to track this, I will change back to deleteLedger in this patch. Thanks for point out. - Fangmin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8141/#review13823 ----------------------------------------------------------- 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 > >
