You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Alessandro Presta <al...@fb.com> on 2012/07/16 15:56:40 UTC
Review Request: Server-side combiner
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5986/
-----------------------------------------------------------
Review request for giraph and Avery Ching.
Description
-------
Uses the combiner for incoming messages on the server.
This addresses bug GIRAPH-224.
https://issues.apache.org/jira/browse/GIRAPH-224
Diffs
-----
http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1357172
Diff: https://reviews.apache.org/r/5986/diff/
Testing
-------
mvn verify
Thanks,
Alessandro Presta
Re: Review Request: Server-side combiner
Posted by Alessandro Presta <al...@fb.com>.
> On July 16, 2012, 6:49 p.m., Avery Ching wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java, lines 132-160
> > <https://reviews.apache.org/r/5986/diff/1/?file=123283#file123283line132>
> >
> > Maybe ServerData should own the combiner? It's a little strange that the request has the combiner. This only works because the combiner is created upon reflection when the request is received.
Agreed, I guess I had a limited understanding of the messaging code when I wrote it...
Since doRequest() is dealing with synchronization, and ServerData is just a container, I guess I'll just add a getCombiner() to ServerData and let doRequest() use it. Ok?
- Alessandro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5986/#review9191
-----------------------------------------------------------
On July 16, 2012, 1:56 p.m., Alessandro Presta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5986/
> -----------------------------------------------------------
>
> (Updated July 16, 2012, 1:56 p.m.)
>
>
> Review request for giraph and Avery Ching.
>
>
> Description
> -------
>
> Uses the combiner for incoming messages on the server.
>
>
> This addresses bug GIRAPH-224.
> https://issues.apache.org/jira/browse/GIRAPH-224
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1357172
>
> Diff: https://reviews.apache.org/r/5986/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Alessandro Presta
>
>
Re: Review Request: Server-side combiner
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5986/#review9191
-----------------------------------------------------------
http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java
<https://reviews.apache.org/r/5986/#comment19672>
Maybe ServerData should own the combiner? It's a little strange that the request has the combiner. This only works because the combiner is created upon reflection when the request is received.
http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java
<https://reviews.apache.org/r/5986/#comment19670>
This should also be done in the non-reflection constructor.
- Avery Ching
On July 16, 2012, 1:56 p.m., Alessandro Presta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5986/
> -----------------------------------------------------------
>
> (Updated July 16, 2012, 1:56 p.m.)
>
>
> Review request for giraph and Avery Ching.
>
>
> Description
> -------
>
> Uses the combiner for incoming messages on the server.
>
>
> This addresses bug GIRAPH-224.
> https://issues.apache.org/jira/browse/GIRAPH-224
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1357172
>
> Diff: https://reviews.apache.org/r/5986/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Alessandro Presta
>
>
Re: Review Request: Server-side combiner
Posted by Alessandro Presta <al...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5986/
-----------------------------------------------------------
(Updated July 16, 2012, 7:31 p.m.)
Review request for giraph and Avery Ching.
Changes
-------
Moved combiner instance to ServerData
Description
-------
Uses the combiner for incoming messages on the server.
This addresses bug GIRAPH-224.
https://issues.apache.org/jira/browse/GIRAPH-224
Diffs (updated)
-----
http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1362205
http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1362205
http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ServerData.java 1362205
http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1362205
http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1362205
Diff: https://reviews.apache.org/r/5986/diff/
Testing
-------
mvn verify
Thanks,
Alessandro Presta