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