You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Ivan Kelly <iv...@apache.org> on 2012/12/03 12:34:05 UTC
Re: Review Request: [BOOKKEEPER-205] Implement a MetaStore based ledger
manager for bookkeeper client.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8035/#review13964
-----------------------------------------------------------
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
<https://reviews.apache.org/r/8035/#comment29872>
Create another JIRA for this TODO
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
<https://reviews.apache.org/r/8035/#comment29875>
There's a problem here. If you only read maxEntriesPerScan in each scan, then when you pass msActiveLedgers to doGC, it may not contain all active ledger ids (if there are more ledgers than maxEntriesPerScan). This will result in any ledger which were not in the scan, but which do exist, being gc'd.
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
<https://reviews.apache.org/r/8035/#comment29873>
From ZooKeeper? Surely from metastore
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
<https://reviews.apache.org/r/8035/#comment29874>
Again, the zk reference is incorrect.
- Ivan Kelly
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
>
>
Re: Review Request: [BOOKKEEPER-205] Implement a MetaStore based ledger
manager for bookkeeper client.
Posted by Jiannan Wang <ji...@yahoo-inc.com>.
> On Dec. 3, 2012, 11:34 a.m., Ivan Kelly wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java, line 596
> > <https://reviews.apache.org/r/8035/diff/1/?file=189045#file189045line596>
> >
> > From ZooKeeper? Surely from metastore
Yeah, u r right.
> On Dec. 3, 2012, 11:34 a.m., Ivan Kelly wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java, line 618
> > <https://reviews.apache.org/r/8035/diff/1/?file=189045#file189045line618>
> >
> > Again, the zk reference is incorrect.
will change the name
> On Dec. 3, 2012, 11:34 a.m., Ivan Kelly wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java, line 576
> > <https://reviews.apache.org/r/8035/diff/1/?file=189045#file189045line576>
> >
> > There's a problem here. If you only read maxEntriesPerScan in each scan, then when you pass msActiveLedgers to doGC, it may not contain all active ledger ids (if there are more ledgers than maxEntriesPerScan). This will result in any ledger which were not in the scan, but which do exist, being gc'd.
We scan all the ledger ID in order, and at each round only maxEntriesPerScan are scanned (again, it's order), so I think the situation you described will not happen.
> On Dec. 3, 2012, 11:34 a.m., Ivan Kelly wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java, line 104
> > <https://reviews.apache.org/r/8035/diff/1/?file=189045#file189045line104>
> >
> > Create another JIRA for this TODO
> >
Will do it
- Jiannan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8035/#review13964
-----------------------------------------------------------
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
>
>