> On Nov. 13, 2012, 11:21 a.m., Fangmin Lv wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java, > > line 591 > > <https://reviews.apache.org/r/8035/diff/1/?file=189045#file189045line591> > > > > I'm afraid the algorithm can not contain all the ledger id ranges. For > > example: > > Suppose bookie and metastore both contain L1, L2, ..., Ln ledgers > > initially, then during the gc interval L1, L2, L(n-1) were deleted from > > metastore, so only Ln will be scaned out, the range would be [n,n], then > > L1, L2, ..., L(n-1) may never deleted in the feature. I think the > > startLedgerId need to start from 0, also need to check the last range > > which from max ledger id in metastore to bookie max active ledgers.
1. startLedgerId is initial to 0 at the beginning of GC (line 528). For each sub gc round, it needn't to restart from 0. cursor.readEntries() always keeps the cursor moving. 2. Yeah, it's the same problem with line 573, after finish scanning all active ledger id in metastore, further processing is needed. > On Nov. 13, 2012, 11:21 a.m., Fangmin Lv wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java, > > line 582 > > <https://reviews.apache.org/r/8035/diff/1/?file=189045#file189045line582> > > > > I think this is not correct, need to change to > > > > if (!entries.hasNext()) good catch > On Nov. 13, 2012, 11:21 a.m., Fangmin Lv wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java, > > line 573 > > <https://reviews.apache.org/r/8035/diff/1/?file=189045#file189045line573> > > > > It seems miss the case that all ledgers were deleted, and need to gc > > all bookie active ledgers. good catch! - Jiannan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8035/#review13389 ----------------------------------------------------------- On Nov. 13, 2012, 4:53 a.m., Jiannan Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8035/ > ----------------------------------------------------------- > > (Updated Nov. 13, 2012, 4:53 a.m.) > > > Review request for bookkeeper. > > > Description > ------- > > Implement a MetaStore based ledger manager for bookkeeper client. > > > This addresses bug BOOKKEEPER-205. > https://issues.apache.org/jira/browse/BOOKKEEPER-205 > > > Diffs > ----- > > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java > e6a3807 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java > 2692fde > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java > PRE-CREATION > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java > 16c0276 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/metastore/TestMetaStore.java > d528a18 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/BookieLedgerIndexTest.java > ead3494 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java > 2f39536 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/MultiLedgerManagerMultiDigestTestCase.java > 9630d46 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/MultiLedgerManagerTestCase.java > dd3450c > > Diff: https://reviews.apache.org/r/8035/diff/ > > > Testing > ------- > > > Thanks, > > Jiannan Wang > >
