You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by fp...@apache.org on 2013/05/31 12:41:25 UTC
Review Request: BOOKKEEPER-592: allow application to recommend ledger data
locality
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11574/
-----------------------------------------------------------
Review request for bookkeeper.
Description
-------
BOOKKEEPER-592: allow application to recommend ledger data locality
This addresses bug BOOKKEEPER-592.
https://issues.apache.org/jira/browse/BOOKKEEPER-592
Diffs
-----
/bookkeeper-server/pom.xml 1488142
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 1488142
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookieWatcher.java 1488142
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultEnsemblePlacementPolicy.java PRE-CREATION
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/EnsemblePlacementPolicy.java PRE-CREATION
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java 1488142
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java 1488142
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java PRE-CREATION
/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java 1488142
/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java 1488142
/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecoveryTest.java 1488142
/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java PRE-CREATION
/bookkeeper-server/src/test/java/org/apache/bookkeeper/util/StaticDNSResolver.java PRE-CREATION
Diff: https://reviews.apache.org/r/11574/diff/
Testing
-------
Thanks,
fpj
Re: Review Request: BOOKKEEPER-592: allow application to recommend ledger
data locality
Posted by Sijie Guo <gu...@gmail.com>.
> On June 4, 2013, 8:05 p.m., fpj wrote:
> > This patch looks good, I just have a few minor issues, typical stuff. Also, as usual, I'd like to have some documentation about it. If you don't want to include it in this patch, please create a jira and mark it as a blocker for 4.3.0.
will address your comments ASAP. thanks for reviewing.
- Sijie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11574/#review21416
-----------------------------------------------------------
On May 31, 2013, 10:41 a.m., fpj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11574/
> -----------------------------------------------------------
>
> (Updated May 31, 2013, 10:41 a.m.)
>
>
> Review request for bookkeeper.
>
>
> Description
> -------
>
> BOOKKEEPER-592: allow application to recommend ledger data locality
>
>
> This addresses bug BOOKKEEPER-592.
> https://issues.apache.org/jira/browse/BOOKKEEPER-592
>
>
> Diffs
> -----
>
> /bookkeeper-server/pom.xml 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookieWatcher.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultEnsemblePlacementPolicy.java PRE-CREATION
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/EnsemblePlacementPolicy.java PRE-CREATION
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java PRE-CREATION
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java 1488142
> /bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecoveryTest.java 1488142
> /bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java PRE-CREATION
> /bookkeeper-server/src/test/java/org/apache/bookkeeper/util/StaticDNSResolver.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/11574/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> fpj
>
>
Re: Review Request: BOOKKEEPER-592: allow application to recommend ledger
data locality
Posted by fp...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11574/#review21416
-----------------------------------------------------------
This patch looks good, I just have a few minor issues, typical stuff. Also, as usual, I'd like to have some documentation about it. If you don't want to include it in this patch, please create a jira and mark it as a blocker for 4.3.0.
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java
<https://reviews.apache.org/r/11574/#comment44366>
... to implement other algorithms.
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java
<https://reviews.apache.org/r/11574/#comment44348>
chosenBookies
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java
<https://reviews.apache.org/r/11574/#comment44364>
fix or new jira, pls
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java
<https://reviews.apache.org/r/11574/#comment44339>
This should be a warning, no? It would be good to warn ops that there aren't enough bookies even in the case we end up returning a randomly selected one.
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java
<https://reviews.apache.org/r/11574/#comment44365>
fix or new jira, please
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java
<https://reviews.apache.org/r/11574/#comment44354>
This block is ok. (Ignore this comment, this is for myself.)
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java
<https://reviews.apache.org/r/11574/#comment44353>
It doesn't look like we need to shuffle every time. Given that we won't be doing this often, it is possibly ok if we keep it like this, though.
- fpj
On May 31, 2013, 10:41 a.m., fpj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11574/
> -----------------------------------------------------------
>
> (Updated May 31, 2013, 10:41 a.m.)
>
>
> Review request for bookkeeper.
>
>
> Description
> -------
>
> BOOKKEEPER-592: allow application to recommend ledger data locality
>
>
> This addresses bug BOOKKEEPER-592.
> https://issues.apache.org/jira/browse/BOOKKEEPER-592
>
>
> Diffs
> -----
>
> /bookkeeper-server/pom.xml 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookieWatcher.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultEnsemblePlacementPolicy.java PRE-CREATION
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/EnsemblePlacementPolicy.java PRE-CREATION
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java PRE-CREATION
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java 1488142
> /bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java 1488142
> /bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecoveryTest.java 1488142
> /bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java PRE-CREATION
> /bookkeeper-server/src/test/java/org/apache/bookkeeper/util/StaticDNSResolver.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/11574/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> fpj
>
>