----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12174/#review23544 -----------------------------------------------------------
Looks pretty close. Just a couple of minor things, then let's get this in. giraph-core/src/main/java/org/apache/giraph/comm/SendCache.java <https://reviews.apache.org/r/12174/#comment47440> This can be private as before with a method to get the keyset (i.e. Set<WorkerInfo> getWorkerInfos()). giraph-core/src/main/java/org/apache/giraph/comm/SendCache.java <https://reviews.apache.org/r/12174/#comment47441> Note that this comment isn't necessary as per the checkstyle rules (when the method is get*, where the member variable is *). giraph-core/src/main/java/org/apache/giraph/comm/SendMessageToAllCache.java <https://reviews.apache.org/r/12174/#comment47443> missing " i.e. "one-to-all" giraph-core/src/main/java/org/apache/giraph/comm/SendMessageToAllCache.java <https://reviews.apache.org/r/12174/#comment47448> I think I like flush being part of the cache, nice. giraph-core/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java <https://reviews.apache.org/r/12174/#comment47447> This is fixing a bug correct? If so shouldn't we insure that it's fixed at the source? I.e. when we construct the request we should have conf in the constructor? giraph-core/src/main/java/org/apache/giraph/comm/requests/SendWorkerOneToAllMessagesRequest.java <https://reviews.apache.org/r/12174/#comment47445> Why not do this logic the same way that SendWorkerRequestMessages does (i.e. via addPartitionMessages()? - Avery Ching On July 16, 2013, 11:33 p.m., Bingjing Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12174/ > ----------------------------------------------------------- > > (Updated July 16, 2013, 11:33 p.m.) > > > Review request for giraph. > > > Bugs: GIRAPH-701 > https://issues.apache.org/jira/browse/GIRAPH-701 > > > Repository: giraph-git > > > Description > ------- > > Add "one-to-all" message sending strategy. > Now when sendMessageToAllEdges is invoked, Giraph may apply "one-to-all" > message sending strategy. > To enable it, use conf.enableOneToAllMsgSending() > > > Diffs > ----- > > giraph-core/src/main/java/org/apache/giraph/bsp/BspService.java ff3f06d > giraph-core/src/main/java/org/apache/giraph/comm/SendCache.java 92d0926 > giraph-core/src/main/java/org/apache/giraph/comm/SendMessageCache.java > 2eeac18 > giraph-core/src/main/java/org/apache/giraph/comm/SendMessageToAllCache.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/comm/ServerData.java a50f673 > > giraph-core/src/main/java/org/apache/giraph/comm/WorkerClientRequestProcessor.java > 89fb3e4 > > giraph-core/src/main/java/org/apache/giraph/comm/messages/OneMessagePerVertexStore.java > bedaf48 > > giraph-core/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java > 7ce0083 > giraph-core/src/main/java/org/apache/giraph/comm/requests/RequestType.java > 4129fb8 > > giraph-core/src/main/java/org/apache/giraph/comm/requests/SendWorkerOneToAllMessagesRequest.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/conf/GiraphConfiguration.java > 21cf245 > giraph-core/src/main/java/org/apache/giraph/conf/GiraphConstants.java > c4cc96f > giraph-core/src/main/java/org/apache/giraph/counters/GiraphStats.java > 3f25508 > giraph-core/src/main/java/org/apache/giraph/graph/Computation.java 87d5879 > giraph-core/src/main/java/org/apache/giraph/graph/ComputeCallable.java > a9bf3fd > giraph-core/src/main/java/org/apache/giraph/graph/GlobalStats.java f3cbea2 > giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java > 1d3cff0 > giraph-core/src/main/java/org/apache/giraph/metrics/MetricNames.java > cc237ac > giraph-core/src/main/java/org/apache/giraph/partition/PartitionStats.java > b8eeca9 > > giraph-core/src/main/java/org/apache/giraph/utils/ByteArrayOneToAllMessages.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/utils/ByteArrayVertexIdData.java > 26c547b > giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java > 52bac3f > giraph-core/src/test/java/org/apache/giraph/comm/RequestTest.java 2e60c09 > giraph-core/src/test/java/org/apache/giraph/utils/MockUtils.java bc5b5e2 > > giraph-examples/src/test/java/org/apache/giraph/examples/SimpleTriangleClosingComputationTest.java > 7346745 > > Diff: https://reviews.apache.org/r/12174/diff/ > > > Testing > ------- > > Did > mvn clean verify > mvn clean test > > > Thanks, > > Bingjing Zhang > >
