You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Maja Kabiljo <ma...@fb.com> on 2012/08/23 20:41:43 UTC

Review Request: Open Netty client and server on master

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6753/
-----------------------------------------------------------

Review request for giraph.


Description
-------

For GIRAPH-273 first thing we need is to open Netty communication on master, make connections to workers and make connections from workers to master.

Since it's already significant amount of code I'm opening a separate issue for it.

This doesn't send any messages for now, but I tested it with some dummy messages and it works. The patch adds all the client/server classes we had for worker for master, and does some redesign so common parts could be reused. Again there are some useNetty checks which we'll be able to remove soon.


This addresses bug GIRAPH-313.
    https://issues.apache.org/jira/browse/GIRAPH-313


Diffs
-----

  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClient.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClientServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClient.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClientServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestInfo.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1375970 
  http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1375970 

Diff: https://reviews.apache.org/r/6753/diff/


Testing
-------

Passes mvn verify and pseudo distributed tests.


Thanks,

Maja Kabiljo


Re: Review Request: Open Netty client and server on master

Posted by Maja Kabiljo <ma...@fb.com>.

> On Aug. 24, 2012, 7:51 a.m., Avery Ching wrote:
> > This looks like a nice refactoring to handle different response for the master and workers.  Looks like a great start to adding direct communication to the master.  I have a few minor things below, but I think we shouldn't be removing the types out of the classes and relying on warning suppression.  I believe the suppresion exists is because of Eclipse showing warnings (Intellij doesn't).  I'd prefer to keep the types there because if we need them later on down the line, it is going to be a mess to add the types in (due to crazy weird dependencies).
> 
> Maja Kabiljo wrote:
>     Avery, thank you for the review.
>     
>     The way I see it, netty client and netty server should be classes which look at requests just as writable objects, and independent of the current algorithm completely. So if we get to a point that we need the types back we are doing something wrong :-) I've taken a look at which warnings do we get. For NettyClient there is only one with RequestEncoder, and that class also doesn't need to be parametrized anymore. For NettyServer we only have the part with registering requests, and that can be done in a different way, we don't necessarily need the instances of requests in order to register them. What do you think?

Also, I see we have @SuppressWarnings("rawtypes") all around the codebase mostly because of two things:
- I extends WritableComparable without a type
- Mapper.Context without types
We can eliminate those warnings easily.


- Maja


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6753/#review10720
-----------------------------------------------------------


On Aug. 23, 2012, 6:41 p.m., Maja Kabiljo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6753/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2012, 6:41 p.m.)
> 
> 
> Review request for giraph.
> 
> 
> Description
> -------
> 
> For GIRAPH-273 first thing we need is to open Netty communication on master, make connections to workers and make connections from workers to master.
> 
> Since it's already significant amount of code I'm opening a separate issue for it.
> 
> This doesn't send any messages for now, but I tested it with some dummy messages and it works. The patch adds all the client/server classes we had for worker for master, and does some redesign so common parts could be reused. Again there are some useNetty checks which we'll be able to remove soon.
> 
> 
> This addresses bug GIRAPH-313.
>     https://issues.apache.org/jira/browse/GIRAPH-313
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestInfo.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1375970 
> 
> Diff: https://reviews.apache.org/r/6753/diff/
> 
> 
> Testing
> -------
> 
> Passes mvn verify and pseudo distributed tests.
> 
> 
> Thanks,
> 
> Maja Kabiljo
> 
>


Re: Review Request: Open Netty client and server on master

Posted by Maja Kabiljo <ma...@fb.com>.

> On Aug. 24, 2012, 7:51 a.m., Avery Ching wrote:
> > This looks like a nice refactoring to handle different response for the master and workers.  Looks like a great start to adding direct communication to the master.  I have a few minor things below, but I think we shouldn't be removing the types out of the classes and relying on warning suppression.  I believe the suppresion exists is because of Eclipse showing warnings (Intellij doesn't).  I'd prefer to keep the types there because if we need them later on down the line, it is going to be a mess to add the types in (due to crazy weird dependencies).

Avery, thank you for the review.

The way I see it, netty client and netty server should be classes which look at requests just as writable objects, and independent of the current algorithm completely. So if we get to a point that we need the types back we are doing something wrong :-) I've taken a look at which warnings do we get. For NettyClient there is only one with RequestEncoder, and that class also doesn't need to be parametrized anymore. For NettyServer we only have the part with registering requests, and that can be done in a different way, we don't necessarily need the instances of requests in order to register them. What do you think?


> On Aug. 24, 2012, 7:51 a.m., Avery Ching wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java, lines 21-29
> > <https://reviews.apache.org/r/6753/diff/1/?file=144826#file144826line21>
> >
> >     Shouldn't we make this an interface in this case (no implementation of any sort)?

Good idea, I'll switch master and worker requests to interfaces.


- Maja


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6753/#review10720
-----------------------------------------------------------


On Aug. 23, 2012, 6:41 p.m., Maja Kabiljo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6753/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2012, 6:41 p.m.)
> 
> 
> Review request for giraph.
> 
> 
> Description
> -------
> 
> For GIRAPH-273 first thing we need is to open Netty communication on master, make connections to workers and make connections from workers to master.
> 
> Since it's already significant amount of code I'm opening a separate issue for it.
> 
> This doesn't send any messages for now, but I tested it with some dummy messages and it works. The patch adds all the client/server classes we had for worker for master, and does some redesign so common parts could be reused. Again there are some useNetty checks which we'll be able to remove soon.
> 
> 
> This addresses bug GIRAPH-313.
>     https://issues.apache.org/jira/browse/GIRAPH-313
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestInfo.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1375970 
> 
> Diff: https://reviews.apache.org/r/6753/diff/
> 
> 
> Testing
> -------
> 
> Passes mvn verify and pseudo distributed tests.
> 
> 
> Thanks,
> 
> Maja Kabiljo
> 
>


Re: Review Request: Open Netty client and server on master

Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6753/#review10720
-----------------------------------------------------------


This looks like a nice refactoring to handle different response for the master and workers.  Looks like a great start to adding direct communication to the master.  I have a few minor things below, but I think we shouldn't be removing the types out of the classes and relying on warning suppression.  I believe the suppresion exists is because of Eclipse showing warnings (Intellij doesn't).  I'd prefer to keep the types there because if we need them later on down the line, it is going to be a mess to add the types in (due to crazy weird dependencies).


http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java
<https://reviews.apache.org/r/6753/#comment23136>

    Shouldn't we make this an interface in this case (no implementation of any sort)?



http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java
<https://reviews.apache.org/r/6753/#comment23137>

    We should remove the suppresion here instead I think and keep the types.



http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java
<https://reviews.apache.org/r/6753/#comment23138>

    Again, we should remove the suppression here and keep the types.



http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java
<https://reviews.apache.org/r/6753/#comment23139>

    extra line.


- Avery Ching


On Aug. 23, 2012, 6:41 p.m., Maja Kabiljo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6753/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2012, 6:41 p.m.)
> 
> 
> Review request for giraph.
> 
> 
> Description
> -------
> 
> For GIRAPH-273 first thing we need is to open Netty communication on master, make connections to workers and make connections from workers to master.
> 
> Since it's already significant amount of code I'm opening a separate issue for it.
> 
> This doesn't send any messages for now, but I tested it with some dummy messages and it works. The patch adds all the client/server classes we had for worker for master, and does some redesign so common parts could be reused. Again there are some useNetty checks which we'll be able to remove soon.
> 
> 
> This addresses bug GIRAPH-313.
>     https://issues.apache.org/jira/browse/GIRAPH-313
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestInfo.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1375970 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1375970 
> 
> Diff: https://reviews.apache.org/r/6753/diff/
> 
> 
> Testing
> -------
> 
> Passes mvn verify and pseudo distributed tests.
> 
> 
> Thanks,
> 
> Maja Kabiljo
> 
>


Re: Review Request: Open Netty client and server on master

Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6753/#review10788
-----------------------------------------------------------

Ship it!


I'm +1'ed on this.  Thanks for the changes Maja.  This is a great addition of master communication and a nice clean up of some APIs as well.


http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java
<https://reviews.apache.org/r/6753/#comment23244>

    This is a good fix, btw.


- Avery Ching


On Aug. 27, 2012, 8:55 a.m., Maja Kabiljo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6753/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2012, 8:55 a.m.)
> 
> 
> Review request for giraph.
> 
> 
> Description
> -------
> 
> For GIRAPH-273 first thing we need is to open Netty communication on master, make connections to workers and make connections from workers to master.
> 
> Since it's already significant amount of code I'm opening a separate issue for it.
> 
> This doesn't send any messages for now, but I tested it with some dummy messages and it works. The patch adds all the client/server classes we had for worker for master, and does some redesign so common parts could be reused. Again there are some useNetty checks which we'll be able to remove soon.
> 
> 
> This addresses bug GIRAPH-313.
>     https://issues.apache.org/jira/browse/GIRAPH-313
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestInfo.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestType.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1377596 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1377596 
> 
> Diff: https://reviews.apache.org/r/6753/diff/
> 
> 
> Testing
> -------
> 
> Passes mvn verify and pseudo distributed tests.
> 
> 
> Thanks,
> 
> Maja Kabiljo
> 
>


Re: Review Request: Open Netty client and server on master

Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6753/
-----------------------------------------------------------

(Updated Aug. 27, 2012, 8:55 a.m.)


Review request for giraph.


Changes
-------

Avery, it's not formally a downcasting since WorkerRequest and MasterRequest don't extend WritableRequest. But anyway, WorkerRequest and MasterRequest need to have different doRequest signatures. Since on master we don't do anything for now I left that one without parameters, but in the end we should have something like WorkerServerData and MasterServerData. And those two classes will have different set of methods.
One thing we could do is have RequestServerHandler<R extends WritableRequest>, and then have 
R request = (R) e.getMessage();
on line 74 in RequestServerHandler. Looks better, but then we can't have WorkerRequest and MasterRequest as interfaces.
Or as I changed now to RequestServerHandler<R>, you can take a look.
If you have some better suggestion I'd be happy to hear/discuss it.

Eli, I'm not sure how things you are saying apply here. ReflectionUtils is used for user defined classes. I'm not removing generics from any of those, I'm just making the request communication working in a general way. Please comment further.


Description
-------

For GIRAPH-273 first thing we need is to open Netty communication on master, make connections to workers and make connections from workers to master.

Since it's already significant amount of code I'm opening a separate issue for it.

This doesn't send any messages for now, but I tested it with some dummy messages and it works. The patch adds all the client/server classes we had for worker for master, and does some redesign so common parts could be reused. Again there are some useNetty checks which we'll be able to remove soon.


This addresses bug GIRAPH-313.
    https://issues.apache.org/jira/browse/GIRAPH-313


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClient.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClientServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClient.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClientServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestInfo.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestType.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1377596 
  http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1377596 

Diff: https://reviews.apache.org/r/6753/diff/


Testing
-------

Passes mvn verify and pseudo distributed tests.


Thanks,

Maja Kabiljo


Re: Review Request: Open Netty client and server on master

Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6753/#review10760
-----------------------------------------------------------


Everything looks fine, except the downcasting...would be great if we could avoid that.  Can fix this?


http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java
<https://reviews.apache.org/r/6753/#comment23198>

    Downcasting is a little yucky.  Can we avoid this?



http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java
<https://reviews.apache.org/r/6753/#comment23197>

    Downcasting is a little yucky.  Can we avoid this?


- Avery Ching


On Aug. 24, 2012, 1:33 p.m., Maja Kabiljo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6753/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2012, 1:33 p.m.)
> 
> 
> Review request for giraph.
> 
> 
> Description
> -------
> 
> For GIRAPH-273 first thing we need is to open Netty communication on master, make connections to workers and make connections from workers to master.
> 
> Since it's already significant amount of code I'm opening a separate issue for it.
> 
> This doesn't send any messages for now, but I tested it with some dummy messages and it works. The patch adds all the client/server classes we had for worker for master, and does some redesign so common parts could be reused. Again there are some useNetty checks which we'll be able to remove soon.
> 
> 
> This addresses bug GIRAPH-313.
>     https://issues.apache.org/jira/browse/GIRAPH-313
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestInfo.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestType.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1376130 
> 
> Diff: https://reviews.apache.org/r/6753/diff/
> 
> 
> Testing
> -------
> 
> Passes mvn verify and pseudo distributed tests.
> 
> 
> Thanks,
> 
> Maja Kabiljo
> 
>


Re: Review Request: Open Netty client and server on master

Posted by Eli Reisman <in...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6753/#review10767
-----------------------------------------------------------


I think the reason we need to keep the wildcard generics like WritableRequest<?, ?, ?, ?> is that wildcard types without bounds have special properties. You can't use the generic type info when declared this way, but the object's type info is not lost when declared and referenced this way. Its the only declaration with these weird properties in Java I think. You see GraphMapper declared this way sometimes in the tests and GenericTypeValidator in GiraphRunner. This is the way ReflectionUtils can climb the type hierarchy without losing type info. If we go to raw types, or bounded wildcards we lose that capability for some reason. So I think that stuff has to stay in, even though its annoying to type and seems needless.


- Eli Reisman


On Aug. 24, 2012, 1:33 p.m., Maja Kabiljo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6753/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2012, 1:33 p.m.)
> 
> 
> Review request for giraph.
> 
> 
> Description
> -------
> 
> For GIRAPH-273 first thing we need is to open Netty communication on master, make connections to workers and make connections from workers to master.
> 
> Since it's already significant amount of code I'm opening a separate issue for it.
> 
> This doesn't send any messages for now, but I tested it with some dummy messages and it works. The patch adds all the client/server classes we had for worker for master, and does some redesign so common parts could be reused. Again there are some useNetty checks which we'll be able to remove soon.
> 
> 
> This addresses bug GIRAPH-313.
>     https://issues.apache.org/jira/browse/GIRAPH-313
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClient.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClientServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterServer.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestInfo.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestType.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1376130 
>   http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1376130 
> 
> Diff: https://reviews.apache.org/r/6753/diff/
> 
> 
> Testing
> -------
> 
> Passes mvn verify and pseudo distributed tests.
> 
> 
> Thanks,
> 
> Maja Kabiljo
> 
>


Re: Review Request: Open Netty client and server on master

Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6753/
-----------------------------------------------------------

(Updated Aug. 24, 2012, 1:33 p.m.)


Review request for giraph.


Changes
-------

Addressed some of Avery's comments. I still kept the types in client and server, but removed the reasons which were causing warnings. We can discuss futherer whether types should be there or not.


Description
-------

For GIRAPH-273 first thing we need is to open Netty communication on master, make connections to workers and make connections from workers to master.

Since it's already significant amount of code I'm opening a separate issue for it.

This doesn't send any messages for now, but I tested it with some dummy messages and it works. The patch adds all the client/server classes we had for worker for master, and does some redesign so common parts could be reused. Again there are some useNetty checks which we'll be able to remove soon.


This addresses bug GIRAPH-313.
    https://issues.apache.org/jira/browse/GIRAPH-313


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClient.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterClientServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/MasterServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClient.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterClientServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyMasterServer.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestInfo.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestType.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequest.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerRequestServerHandler.java PRE-CREATION 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1376130 
  http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1376130 

Diff: https://reviews.apache.org/r/6753/diff/


Testing
-------

Passes mvn verify and pseudo distributed tests.


Thanks,

Maja Kabiljo