-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7308/#review11964
-----------------------------------------------------------
src/main/java/org/apache/giraph/comm/SendMessageCache.java
<https://reviews.apache.org/r/7308/#comment25497>
Would be nice to change this to
"for all workers"
src/main/java/org/apache/giraph/comm/messages/SimpleMessageStore.java
<https://reviews.apache.org/r/7308/#comment25498>
Thank you for fixing this!
src/main/java/org/apache/giraph/comm/messages/SimpleMessageStore.java
<https://reviews.apache.org/r/7308/#comment25499>
Good catch!
src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java
<https://reviews.apache.org/r/7308/#comment25500>
I see what you are doing, but couldn't we instead have another method
getInetSocketAddress(WorkerInfo workerInfo)
that does this instead of sending in an unexpected partition id? This seems a little
hacky.
At the very least, we should modify the javadoc for @param partitionId to reflect that
we do expect USE_WORKERINFO_ADDRESS in some cases.
src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java
<https://reviews.apache.org/r/7308/#comment25502>
This could just use the new method getInetSocketAddress(entry.getKey) I suggested earlier.
src/main/java/org/apache/giraph/comm/requests/SendPartitionCurrentMessagesRequest.java
<https://reviews.apache.org/r/7308/#comment25505>
Please remove.
src/main/java/org/apache/giraph/comm/requests/SendPartitionCurrentMessagesRequest.java
<https://reviews.apache.org/r/7308/#comment25503>
getConf().createVertexId() instead please, shouldn't use BspUtils anymore.
src/main/java/org/apache/giraph/comm/requests/SendPartitionCurrentMessagesRequest.java
<https://reviews.apache.org/r/7308/#comment25504>
Please no BspUtils.
src/main/java/org/apache/giraph/comm/requests/SendWorkerMessagesRequest.java
<https://reviews.apache.org/r/7308/#comment25506>
Messages sent for one or more partitions?
- Avery Ching
On Sept. 27, 2012, 5:32 a.m., Avery Ching wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7308/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2012, 5:32 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Review of Eli's GIRAPH-328.5.patch. Reviewboard helps me to be more precise about my
comments.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/giraph/comm/SendMessageCache.java 3d38712
> src/main/java/org/apache/giraph/comm/messages/SimpleMessageStore.java ce0e64c
> src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java f3a8d4f
> src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692
> src/main/java/org/apache/giraph/comm/requests/SendPartitionCurrentMessagesRequest.java
7cae1e2
> src/main/java/org/apache/giraph/comm/requests/SendPartitionMessagesRequest.java 1b3a435
> src/main/java/org/apache/giraph/comm/requests/SendWorkerMessagesRequest.java PRE-CREATION
> src/main/java/org/apache/giraph/graph/BspServiceMaster.java 18b5387
> src/main/java/org/apache/giraph/graph/WorkerInfo.java 0224d87
> src/test/java/org/apache/giraph/comm/RequestFailureTest.java 02f699f
> src/test/java/org/apache/giraph/comm/RequestTest.java aa0ddd2
>
> Diff: https://reviews.apache.org/r/7308/diff/
>
>
> Testing
> -------
>
> mvn clean verify
>
>
> Thanks,
>
> Avery Ching
>
>
|