You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Eugene Koontz <ek...@hiro-tan.org> on 2012/08/06 20:15:42 UTC
Re: Review Request: GIRAPH-262 Netty optimizations and bandwidth tracking
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6008/#review9890
-----------------------------------------------------------
The channel-pool seems like a good idea and so does the bandwidth metrics, but how about a JIRA for each one?
http://svn.apache.org/repos/asf/giraph/trunk/pom.xml
<https://reviews.apache.org/r/6008/#comment21018>
added this as a separate JIRA: https://issues.apache.org/jira/browse/GIRAPH-280
http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java
<https://reviews.apache.org/r/6008/#comment21019>
The sendBufferSize and receiveBufferSize parameterizations should go in a new JIRA : added GIRAPH-281.
http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java
<https://reviews.apache.org/r/6008/#comment21017>
For consistency with other pipeline components, use new ByteCounter() here.
- Eugene Koontz
On July 17, 2012, 7:33 a.m., Avery Ching wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6008/
> -----------------------------------------------------------
>
> (Updated July 17, 2012, 7:33 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> * Makes netty the default instead of HadoopRPC
> * Added optimization to handle requests locally when possible rather than go over the network
> * Added TimedLogger to print only within a given time period
> * Added optimization for using multiple channels between clients/servers when bandwidth is limited per connection
> * Added ByteCounter to track bandwidth across Netty (can be later integrated with GIRAPH-232).
> * Upgraded rat to 0.8 and excluded iml files (Intellij Idea)
> * Set 32 instead of 64 as the default maximum netty threads
>
>
> This addresses bug GIRAPH-262.
> https://issues.apache.org/jira/browse/GIRAPH-262
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/pom.xml 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ByteCounter.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ChannelRotater.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ResponseClientHandler.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/TimedLogger.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1362298
>
> Diff: https://reviews.apache.org/r/6008/diff/
>
>
> Testing
> -------
>
> Passed mvn verify. Used for the Hadoop Summit presentation results.
>
>
> Thanks,
>
> Avery Ching
>
>
Re: Review Request: GIRAPH-262 Netty optimizations and bandwidth tracking
Posted by Avery Ching <av...@gmail.com>.
> On Aug. 6, 2012, 6:15 p.m., Eugene Koontz wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java, line 112
> > <https://reviews.apache.org/r/6008/diff/1/?file=123493#file123493line112>
> >
> > For consistency with other pipeline components, use new ByteCounter() here.
>
> Avery Ching wrote:
> I need the reference here so that I can print the metrics periodically in the client. See NettyClient.java:237.
>
> Eugene Koontz wrote:
> I see, makes sense, thanks.
Thanks for taking a look at this Eugene. Do you have any more comments, or is this ready to go?
- Avery
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6008/#review9890
-----------------------------------------------------------
On Aug. 7, 2012, 1:04 a.m., Avery Ching wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6008/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2012, 1:04 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> * Makes netty the default instead of HadoopRPC
> * Added optimization to handle requests locally when possible rather than go over the network
> * Added TimedLogger to print only within a given time period
> * Added optimization for using multiple channels between clients/servers when bandwidth is limited per connection
> * Added ByteCounter to track bandwidth across Netty (can be later integrated with GIRAPH-232).
> * Upgraded rat to 0.8 and excluded iml files (Intellij Idea)
> * Set 32 instead of 64 as the default maximum netty threads
>
>
> This addresses bug GIRAPH-262.
> https://issues.apache.org/jira/browse/GIRAPH-262
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/pom.xml 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ByteCounter.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ChannelRotater.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ResponseClientHandler.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/TimedLogger.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1370086
>
> Diff: https://reviews.apache.org/r/6008/diff/
>
>
> Testing
> -------
>
> Passed mvn verify. Used for the Hadoop Summit presentation results.
>
>
> Thanks,
>
> Avery Ching
>
>
Re: Review Request: GIRAPH-262 Netty optimizations and bandwidth tracking
Posted by Eugene Koontz <ek...@hiro-tan.org>.
> On Aug. 6, 2012, 6:15 p.m., Eugene Koontz wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java, line 112
> > <https://reviews.apache.org/r/6008/diff/1/?file=123493#file123493line112>
> >
> > For consistency with other pipeline components, use new ByteCounter() here.
>
> Avery Ching wrote:
> I need the reference here so that I can print the metrics periodically in the client. See NettyClient.java:237.
>
> Eugene Koontz wrote:
> I see, makes sense, thanks.
>
> Avery Ching wrote:
> Thanks for taking a look at this Eugene. Do you have any more comments, or is this ready to go?
Hi Avery,
Thanks a lot for rebasing on top of those 3 new JIRAs.
If you don't mind I feel like there are three JIRAs in here still :)
1. netty optimization #1 shortcircuit to local doRequest() calls
2. netty optimization #2 connection pools
3. bandwidth tracking
I would like to extract two of them and leave one behind as GIRAPH-262 (so it would be 1. (netty optimization #1))
The reason I'm being so picky about it is that I've modified a lot of the same code in my GIRAPH-211 patch. So I want to carefully merge your enhancements into there.
I am happy to extract 2 and 3 out today as I did yesterday for GIRAPH-280-282, if you don't mind kindly rebasing your patch on them.
-Eugene
- Eugene
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6008/#review9890
-----------------------------------------------------------
On Aug. 7, 2012, 1:04 a.m., Avery Ching wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6008/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2012, 1:04 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> * Makes netty the default instead of HadoopRPC
> * Added optimization to handle requests locally when possible rather than go over the network
> * Added TimedLogger to print only within a given time period
> * Added optimization for using multiple channels between clients/servers when bandwidth is limited per connection
> * Added ByteCounter to track bandwidth across Netty (can be later integrated with GIRAPH-232).
> * Upgraded rat to 0.8 and excluded iml files (Intellij Idea)
> * Set 32 instead of 64 as the default maximum netty threads
>
>
> This addresses bug GIRAPH-262.
> https://issues.apache.org/jira/browse/GIRAPH-262
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/pom.xml 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ByteCounter.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ChannelRotater.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ResponseClientHandler.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/TimedLogger.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1370086
>
> Diff: https://reviews.apache.org/r/6008/diff/
>
>
> Testing
> -------
>
> Passed mvn verify. Used for the Hadoop Summit presentation results.
>
>
> Thanks,
>
> Avery Ching
>
>
Re: Review Request: GIRAPH-262 Netty optimizations and bandwidth tracking
Posted by Eugene Koontz <ek...@hiro-tan.org>.
> On Aug. 6, 2012, 6:15 p.m., Eugene Koontz wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java, line 112
> > <https://reviews.apache.org/r/6008/diff/1/?file=123493#file123493line112>
> >
> > For consistency with other pipeline components, use new ByteCounter() here.
>
> Avery Ching wrote:
> I need the reference here so that I can print the metrics periodically in the client. See NettyClient.java:237.
I see, makes sense, thanks.
- Eugene
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6008/#review9890
-----------------------------------------------------------
On Aug. 7, 2012, 1:04 a.m., Avery Ching wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6008/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2012, 1:04 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> * Makes netty the default instead of HadoopRPC
> * Added optimization to handle requests locally when possible rather than go over the network
> * Added TimedLogger to print only within a given time period
> * Added optimization for using multiple channels between clients/servers when bandwidth is limited per connection
> * Added ByteCounter to track bandwidth across Netty (can be later integrated with GIRAPH-232).
> * Upgraded rat to 0.8 and excluded iml files (Intellij Idea)
> * Set 32 instead of 64 as the default maximum netty threads
>
>
> This addresses bug GIRAPH-262.
> https://issues.apache.org/jira/browse/GIRAPH-262
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/pom.xml 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ByteCounter.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ChannelRotater.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ResponseClientHandler.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1370086
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/TimedLogger.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1370086
>
> Diff: https://reviews.apache.org/r/6008/diff/
>
>
> Testing
> -------
>
> Passed mvn verify. Used for the Hadoop Summit presentation results.
>
>
> Thanks,
>
> Avery Ching
>
>
Re: Review Request: GIRAPH-262 Netty optimizations and bandwidth tracking
Posted by Avery Ching <av...@gmail.com>.
> On Aug. 6, 2012, 6:15 p.m., Eugene Koontz wrote:
> > The channel-pool seems like a good idea and so does the bandwidth metrics, but how about a JIRA for each one?
While I understand why you are saying this, I felt these changes were pretty close in nature as a set of optimizations. They also somewhat depend on each other, so I could split up into patches, but would have to rebase after each commit I think.
> On Aug. 6, 2012, 6:15 p.m., Eugene Koontz wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java, line 112
> > <https://reviews.apache.org/r/6008/diff/1/?file=123493#file123493line112>
> >
> > For consistency with other pipeline components, use new ByteCounter() here.
I need the reference here so that I can print the metrics periodically in the client. See NettyClient.java:237.
> On Aug. 6, 2012, 6:15 p.m., Eugene Koontz wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/pom.xml, line 409
> > <https://reviews.apache.org/r/6008/diff/1/?file=123489#file123489line409>
> >
> > added this as a separate JIRA: https://issues.apache.org/jira/browse/GIRAPH-280
Committed.
> On Aug. 6, 2012, 6:15 p.m., Eugene Koontz wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java, line 79
> > <https://reviews.apache.org/r/6008/diff/1/?file=123493#file123493line79>
> >
> > The sendBufferSize and receiveBufferSize parameterizations should go in a new JIRA : added GIRAPH-281.
I'll put this in GIRAPH-281.
- Avery
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6008/#review9890
-----------------------------------------------------------
On July 17, 2012, 7:33 a.m., Avery Ching wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6008/
> -----------------------------------------------------------
>
> (Updated July 17, 2012, 7:33 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> * Makes netty the default instead of HadoopRPC
> * Added optimization to handle requests locally when possible rather than go over the network
> * Added TimedLogger to print only within a given time period
> * Added optimization for using multiple channels between clients/servers when bandwidth is limited per connection
> * Added ByteCounter to track bandwidth across Netty (can be later integrated with GIRAPH-232).
> * Upgraded rat to 0.8 and excluded iml files (Intellij Idea)
> * Set 32 instead of 64 as the default maximum netty threads
>
>
> This addresses bug GIRAPH-262.
> https://issues.apache.org/jira/browse/GIRAPH-262
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/pom.xml 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ByteCounter.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ChannelRotater.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ResponseClientHandler.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/TimedLogger.java PRE-CREATION
> http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1362298
> http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java 1362298
>
> Diff: https://reviews.apache.org/r/6008/diff/
>
>
> Testing
> -------
>
> Passed mvn verify. Used for the Hadoop Summit presentation results.
>
>
> Thanks,
>
> Avery Ching
>
>