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/10/16 02:24:29 UTC
Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/
-----------------------------------------------------------
Review request for giraph.
Description
-------
In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
This addresses bug GIRAPH-372.
https://issues.apache.org/jira/browse/GIRAPH-372
Diffs
-----
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientServer.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1398379
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1398379
Diff: https://reviews.apache.org/r/7610/diff/
Testing
-------
Passes mvn verify
Thanks,
Maja Kabiljo
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Eli Reisman <in...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/#review12513
-----------------------------------------------------------
This is great work Maja. Now that everyone's addresses are in the same place (master and clients), perhaps objects like InetSocketAddress or especially WorkerInfo could do with a name change, or possibly even reconfiguration into a simpler form? Also, just for my understanding: all host addresses are now on the same ZK children list, and are read by NettyClient's service object from this list, each superstep?
Is there ever a time when a given worker task would not have both a PartitionId -> PartitionOwners map and also now a taskId -> WorkerInfo map? Perhaps these two related things could be wrapped in an object that further cleans up interactions from calling code (various master and worker code like Netty* classes)
I feel like at this point a lot of the tangles in the addressing/messaging setup code is cleaned up in this patch. anything else you can do while you're in there to modularize these interactions would be a big added bonus.
Anyway, this is great. I love when the right side of the diff has lots of empty space! Looks like you shaved lots of lines off those classes!
- Eli Reisman
On Oct. 16, 2012, 12:24 a.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7610/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2012, 12:24 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
>
> Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
>
>
> This addresses bug GIRAPH-372.
> https://issues.apache.org/jira/browse/GIRAPH-372
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientServer.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1398379
>
> Diff: https://reviews.apache.org/r/7610/diff/
>
>
> Testing
> -------
>
> Passes mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/#review12532
-----------------------------------------------------------
Looking good Maja, a few minor changes requested below.
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java
<https://reviews.apache.org/r/7610/#comment26696>
You could move this to CentralizedService since it is used by both the worker and server.
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java
<https://reviews.apache.org/r/7610/#comment26697>
This doesn't appear to have been moved correctly from NettyWorkerClient (i.e. comment, private static, etc.).
- Avery Ching
On Oct. 17, 2012, 9:04 p.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7610/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2012, 9:04 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
>
> Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
>
>
> This addresses bug GIRAPH-372.
> https://issues.apache.org/jira/browse/GIRAPH-372
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/AddressesAndPartitionsWritable.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1399356
>
> Diff: https://reviews.apache.org/r/7610/diff/
>
>
> Testing
> -------
>
> Passes mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/#review12536
-----------------------------------------------------------
2 minor things.
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java
<https://reviews.apache.org/r/7610/#comment26700>
We don't need this right?
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java
<https://reviews.apache.org/r/7610/#comment26701>
extra line missing from original comment =)?
- Avery Ching
On Oct. 17, 2012, 9:11 p.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7610/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2012, 9:11 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
>
> Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
>
>
> This addresses bug GIRAPH-372.
> https://issues.apache.org/jira/browse/GIRAPH-372
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/AddressesAndPartitionsWritable.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1399356
>
> Diff: https://reviews.apache.org/r/7610/diff/
>
>
> Testing
> -------
>
> Passes mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Maja Kabiljo <ma...@fb.com>.
> On Oct. 17, 2012, 9:55 p.m., Avery Ching wrote:
> > +1! Thanks for all the changes Maja!
Thanks for the review, Avery!
Forgot to hit publish along the way... Eli about your comments:
- I wanted to change WorkerInfo name, but it appears in too many places so I thought it's better to do it in a separate Jira.
- Addresses are now written together with partitions owners to the same znode.
- I agree there is more cleanup to be done. Writing WorkerInfos from PartitionOwners is redundant. But again, we can open another issue for that and discuss additional improvements there.
- Maja
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/#review12540
-----------------------------------------------------------
On Oct. 17, 2012, 9:52 p.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7610/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2012, 9:52 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
>
> Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
>
>
> This addresses bug GIRAPH-372.
> https://issues.apache.org/jira/browse/GIRAPH-372
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/AddressesAndPartitionsWritable.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1399356
>
> Diff: https://reviews.apache.org/r/7610/diff/
>
>
> Testing
> -------
>
> Passes mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/#review12540
-----------------------------------------------------------
Ship it!
+1! Thanks for all the changes Maja!
- Avery Ching
On Oct. 17, 2012, 9:52 p.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7610/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2012, 9:52 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
>
> Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
>
>
> This addresses bug GIRAPH-372.
> https://issues.apache.org/jira/browse/GIRAPH-372
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/AddressesAndPartitionsWritable.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1399356
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1399356
>
> Diff: https://reviews.apache.org/r/7610/diff/
>
>
> Testing
> -------
>
> Passes mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/
-----------------------------------------------------------
(Updated Oct. 17, 2012, 9:52 p.m.)
Review request for giraph.
Description
-------
In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
This addresses bug GIRAPH-372.
https://issues.apache.org/jira/browse/GIRAPH-372
Diffs (updated)
-----
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/AddressesAndPartitionsWritable.java PRE-CREATION
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1399356
Diff: https://reviews.apache.org/r/7610/diff/
Testing
-------
Passes mvn verify
Thanks,
Maja Kabiljo
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/
-----------------------------------------------------------
(Updated Oct. 17, 2012, 9:11 p.m.)
Review request for giraph.
Description
-------
In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
This addresses bug GIRAPH-372.
https://issues.apache.org/jira/browse/GIRAPH-372
Diffs (updated)
-----
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedService.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/AddressesAndPartitionsWritable.java PRE-CREATION
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1399356
Diff: https://reviews.apache.org/r/7610/diff/
Testing
-------
Passes mvn verify
Thanks,
Maja Kabiljo
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/
-----------------------------------------------------------
(Updated Oct. 17, 2012, 9:04 p.m.)
Review request for giraph.
Changes
-------
Forgot to make resolveAddress static as in newest code
Description
-------
In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
This addresses bug GIRAPH-372.
https://issues.apache.org/jira/browse/GIRAPH-372
Diffs (updated)
-----
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/AddressesAndPartitionsWritable.java PRE-CREATION
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1399356
Diff: https://reviews.apache.org/r/7610/diff/
Testing
-------
Passes mvn verify
Thanks,
Maja Kabiljo
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/
-----------------------------------------------------------
(Updated Oct. 17, 2012, 7:46 p.m.)
Review request for giraph.
Changes
-------
Rebased, addressed Avery's comments.
Re-run tests (in both modes).
Description
-------
In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
This addresses bug GIRAPH-372.
https://issues.apache.org/jira/browse/GIRAPH-372
Diffs (updated)
-----
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/AddressesAndPartitionsWritable.java PRE-CREATION
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/InputSplitsCallable.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1399356
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1399356
Diff: https://reviews.apache.org/r/7610/diff/
Testing
-------
Passes mvn verify
Thanks,
Maja Kabiljo
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/#review12506
-----------------------------------------------------------
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java
<https://reviews.apache.org/r/7610/#comment26633>
Note that this was only recently added in GIRAPH-360. It seems like it didn't last long :)
Will follow up on GIRAPH-372 JIRA.
- Eugene Koontz
On Oct. 16, 2012, 12:24 a.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7610/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2012, 12:24 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
>
> Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
>
>
> This addresses bug GIRAPH-372.
> https://issues.apache.org/jira/browse/GIRAPH-372
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientServer.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1398379
>
> Diff: https://reviews.apache.org/r/7610/diff/
>
>
> Testing
> -------
>
> Passes mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-372: Write worker addresses to Zookeeper;
move addresses and resolution to NettyClient
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7610/#review12502
-----------------------------------------------------------
Hey this really does simplify things (although it adds a tiny overhead of writing a bit more to ZooKeeper with the chosen workers. Overall though a nice improvement. Could of minor issues below.
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java
<https://reviews.apache.org/r/7610/#comment26622>
Can we get rid of this then?
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java
<https://reviews.apache.org/r/7610/#comment26623>
After we commit GIRAPH-374, this will have a better name I think =)
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java
<https://reviews.apache.org/r/7610/#comment26624>
Boy, this logic gets a lot simpler!
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java
<https://reviews.apache.org/r/7610/#comment26625>
It would be nice if we could somehow could combine this with setting the partitions owners, but this is okay...
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
<https://reviews.apache.org/r/7610/#comment26627>
Minor quip: Can you capitalize
// Get address...
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
<https://reviews.apache.org/r/7610/#comment26628>
// Last address..
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java
<https://reviews.apache.org/r/7610/#comment26626>
I like!
- Avery Ching
On Oct. 16, 2012, 12:24 a.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7610/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2012, 12:24 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> In preparation for GIRAPH-273, we need to have addresses of all workers available, so I write them to Zookeeper along with the master address.
>
> Since address resoultion is needed in both NettyWorkerClient and NettyMasterClient, I moved that to NettyClient, and also added a map taskId->address in there. Now NettyMasterClient and NettyWorkerClient don't need to take care of InetSocketAddresses, just task ids which they want to send messages to.
>
>
> This addresses bug GIRAPH-372.
> https://issues.apache.org/jira/browse/GIRAPH-372
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceMaster.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/bsp/CentralizedServiceWorker.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/MasterClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/WorkerClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientServer.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspService.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/WorkerInfo.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/WritableUtils.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1398379
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1398379
>
> Diff: https://reviews.apache.org/r/7610/diff/
>
>
> Testing
> -------
>
> Passes mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>