-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8035/#review13389
-----------------------------------------------------------



bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
<https://reviews.apache.org/r/8035/#comment28696>

    It seems miss the case that all ledgers were deleted, and need to gc all 
bookie active ledgers.



bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
<https://reviews.apache.org/r/8035/#comment28697>

    I think this is not correct, need to change to 
    
    if (!entries.hasNext()) 



bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
<https://reviews.apache.org/r/8035/#comment28698>

    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.


- Fangmin Lv


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
> 
>

Reply via email to