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