You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Ted Malaska <te...@cloudera.com> on 2013/02/13 18:02:32 UTC
Review Request: Enhance NettyAvroRpcClient and the use of NettyServer to
optionally use compression
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9427/
-----------------------------------------------------------
Review request for Flume.
Description
-------
Enhance NettyAvroRpcClient and the use of NettyServer to optionally use compression
Description: This JIRA will update AvroSource and NettyAvroRpcClient to optionally use ZlibEncoder and ZlibDecoder to compress communications between themselves.
Patch Comment: Added compression options for both client and server.
Note compress has be turn on both client and server for it to work.
Client can have compression with out compression if compression level is set to 0
This addresses bug FLUME-1915.
https://issues.apache.org/jira/browse/FLUME-1915
Diffs
-----
flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java dc18c5d
flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 4bf36e6
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java cf9724c
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java 1e642d8
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 5042d11
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 99ae010
Diff: https://reviews.apache.org/r/9427/diff/
Testing
-------
Yes
Thanks,
Ted Malaska
Re: Review Request: Enhance NettyAvroRpcClient and the use of NettyServer to
optionally use compression
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9427/#review16985
-----------------------------------------------------------
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
<https://reviews.apache.org/r/9427/#comment35985>
Should be "true".equalsIgnoresCase(enableCompressionStr).
- Brock Noland
On Feb. 13, 2013, 5:02 p.m., Ted Malaska wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9427/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 5:02 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Enhance NettyAvroRpcClient and the use of NettyServer to optionally use compression
>
> Description: This JIRA will update AvroSource and NettyAvroRpcClient to optionally use ZlibEncoder and ZlibDecoder to compress communications between themselves.
>
> Patch Comment: Added compression options for both client and server.
> Note compress has be turn on both client and server for it to work.
> Client can have compression with out compression if compression level is set to 0
>
>
> This addresses bug FLUME-1915.
> https://issues.apache.org/jira/browse/FLUME-1915
>
>
> Diffs
> -----
>
> flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java dc18c5d
> flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 4bf36e6
> flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java cf9724c
> flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java 1e642d8
> flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 5042d11
> flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 99ae010
>
> Diff: https://reviews.apache.org/r/9427/diff/
>
>
> Testing
> -------
>
> Yes
>
>
> Thanks,
>
> Ted Malaska
>
>
Re: Review Request: Enhance NettyAvroRpcClient and the use of NettyServer to
optionally use compression
Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9427/#review16982
-----------------------------------------------------------
Hey Ted,
Since it doesn't look like we really have good hooks for specifying headers or dynamically changing the Netty pipeline, the implementation looks good. A few points of feedback:
1. Let's plumb the configuration parameters from the client down to the AbstractRpcSink.configure() method to enable compression at the Avro Sink level
2. How about a unit test for the Avro Sink with compression talking to an Avro Source with compression?
3. It would be great to update the user docs for the AvroSource & AvroSink @ flume-ng-doc/sphinx/FlumeUserGuide.rst and the RpcClient @ flume-ng-doc/sphinx/FlumeDeveloperGuide.rst
4. In order to enable extension in the future (such as support for snappy or some other compression codec), how about we change the config param compression-enabled to compression-type = none | zlib
Does that sound alright?
Regards,
Mike
- Mike Percy
On Feb. 13, 2013, 5:02 p.m., Ted Malaska wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9427/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 5:02 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Enhance NettyAvroRpcClient and the use of NettyServer to optionally use compression
>
> Description: This JIRA will update AvroSource and NettyAvroRpcClient to optionally use ZlibEncoder and ZlibDecoder to compress communications between themselves.
>
> Patch Comment: Added compression options for both client and server.
> Note compress has be turn on both client and server for it to work.
> Client can have compression with out compression if compression level is set to 0
>
>
> This addresses bug FLUME-1915.
> https://issues.apache.org/jira/browse/FLUME-1915
>
>
> Diffs
> -----
>
> flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java dc18c5d
> flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 4bf36e6
> flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java cf9724c
> flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java 1e642d8
> flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 5042d11
> flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 99ae010
>
> Diff: https://reviews.apache.org/r/9427/diff/
>
>
> Testing
> -------
>
> Yes
>
>
> Thanks,
>
> Ted Malaska
>
>
Re: Review Request: Enhance NettyAvroRpcClient and the use of NettyServer to
optionally use compression
Posted by Ted Malaska <te...@cloudera.com>.
> On Feb. 22, 2013, 8:11 a.m., Mike Percy wrote:
> > +1 looks good to me! Ran into some JUnit problem in IntelliJ but I plan on committing this as soon as I can check a couple things from an IDE tomorrow.
Thanks Mike.
- Ted
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9427/#review16926
-----------------------------------------------------------
On Feb. 13, 2013, 5:02 p.m., Ted Malaska wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9427/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 5:02 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Enhance NettyAvroRpcClient and the use of NettyServer to optionally use compression
>
> Description: This JIRA will update AvroSource and NettyAvroRpcClient to optionally use ZlibEncoder and ZlibDecoder to compress communications between themselves.
>
> Patch Comment: Added compression options for both client and server.
> Note compress has be turn on both client and server for it to work.
> Client can have compression with out compression if compression level is set to 0
>
>
> This addresses bug FLUME-1915.
> https://issues.apache.org/jira/browse/FLUME-1915
>
>
> Diffs
> -----
>
> flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java dc18c5d
> flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 4bf36e6
> flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java cf9724c
> flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java 1e642d8
> flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 5042d11
> flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 99ae010
>
> Diff: https://reviews.apache.org/r/9427/diff/
>
>
> Testing
> -------
>
> Yes
>
>
> Thanks,
>
> Ted Malaska
>
>
Re: Review Request: Enhance NettyAvroRpcClient and the use of NettyServer to
optionally use compression
Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9427/#review16926
-----------------------------------------------------------
Ship it!
+1 looks good to me! Ran into some JUnit problem in IntelliJ but I plan on committing this as soon as I can check a couple things from an IDE tomorrow.
- Mike Percy
On Feb. 13, 2013, 5:02 p.m., Ted Malaska wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9427/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 5:02 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Enhance NettyAvroRpcClient and the use of NettyServer to optionally use compression
>
> Description: This JIRA will update AvroSource and NettyAvroRpcClient to optionally use ZlibEncoder and ZlibDecoder to compress communications between themselves.
>
> Patch Comment: Added compression options for both client and server.
> Note compress has be turn on both client and server for it to work.
> Client can have compression with out compression if compression level is set to 0
>
>
> This addresses bug FLUME-1915.
> https://issues.apache.org/jira/browse/FLUME-1915
>
>
> Diffs
> -----
>
> flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java dc18c5d
> flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 4bf36e6
> flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java cf9724c
> flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java 1e642d8
> flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 5042d11
> flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 99ae010
>
> Diff: https://reviews.apache.org/r/9427/diff/
>
>
> Testing
> -------
>
> Yes
>
>
> Thanks,
>
> Ted Malaska
>
>
Re: Review Request: Enhance NettyAvroRpcClient and the use of NettyServer to
optionally use compression
Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9427/#review16959
-----------------------------------------------------------
My only concern is that a client who doesn't know about compression cannot talk to a server with compression enabled. Is there any way to detect that the client is not compressed and handle it accordingly at the server?
That way, all servers can enable compression and maintain backcompat with existing clients.
- Mike Percy
On Feb. 13, 2013, 5:02 p.m., Ted Malaska wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9427/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 5:02 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Enhance NettyAvroRpcClient and the use of NettyServer to optionally use compression
>
> Description: This JIRA will update AvroSource and NettyAvroRpcClient to optionally use ZlibEncoder and ZlibDecoder to compress communications between themselves.
>
> Patch Comment: Added compression options for both client and server.
> Note compress has be turn on both client and server for it to work.
> Client can have compression with out compression if compression level is set to 0
>
>
> This addresses bug FLUME-1915.
> https://issues.apache.org/jira/browse/FLUME-1915
>
>
> Diffs
> -----
>
> flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java dc18c5d
> flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 4bf36e6
> flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java cf9724c
> flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java 1e642d8
> flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 5042d11
> flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 99ae010
>
> Diff: https://reviews.apache.org/r/9427/diff/
>
>
> Testing
> -------
>
> Yes
>
>
> Thanks,
>
> Ted Malaska
>
>
Re: Review Request: Enhance NettyAvroRpcClient and the use of NettyServer to
optionally use compression
Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9427/#review17583
-----------------------------------------------------------
Ship it!
+1, lgtm, I am going to commit this. I have one request for a minor follow-up patch inline.
flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java
<https://reviews.apache.org/r/9427/#comment37291>
Can we file a follow-up JIRA to make this only active when compressionType == none or null/empty? A validation / usability nit.
- Mike Percy
On Feb. 27, 2013, 4:34 a.m., Ted Malaska wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9427/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2013, 4:34 a.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Enhance NettyAvroRpcClient and the use of NettyServer to optionally use compression
>
> Description: This JIRA will update AvroSource and NettyAvroRpcClient to optionally use ZlibEncoder and ZlibDecoder to compress communications between themselves.
>
> Patch Comment: Added compression options for both client and server.
> Note compress has be turn on both client and server for it to work.
> Client can have compression with out compression if compression level is set to 0
>
>
> This addresses bug FLUME-1915.
> https://issues.apache.org/jira/browse/FLUME-1915
>
>
> Diffs
> -----
>
> flume-ng-core/src/main/java/org/apache/flume/sink/AbstractRpcSink.java 52bd49b
> flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java dc18c5d
> flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java b9e59ef
> flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 4bf36e6
> flume-ng-doc/sphinx/FlumeUserGuide.rst 8eb3734
> flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java cf9724c
> flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java 1e642d8
> flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 5042d11
> flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 99ae010
>
> Diff: https://reviews.apache.org/r/9427/diff/
>
>
> Testing
> -------
>
> Yes
>
>
> Thanks,
>
> Ted Malaska
>
>
Re: Review Request: Enhance NettyAvroRpcClient and the use of NettyServer to
optionally use compression
Posted by Ted Malaska <te...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9427/
-----------------------------------------------------------
(Updated Feb. 27, 2013, 4:34 a.m.)
Review request for Flume.
Changes
-------
Addresses all review comments
Description
-------
Enhance NettyAvroRpcClient and the use of NettyServer to optionally use compression
Description: This JIRA will update AvroSource and NettyAvroRpcClient to optionally use ZlibEncoder and ZlibDecoder to compress communications between themselves.
Patch Comment: Added compression options for both client and server.
Note compress has be turn on both client and server for it to work.
Client can have compression with out compression if compression level is set to 0
This addresses bug FLUME-1915.
https://issues.apache.org/jira/browse/FLUME-1915
Diffs (updated)
-----
flume-ng-core/src/main/java/org/apache/flume/sink/AbstractRpcSink.java 52bd49b
flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java dc18c5d
flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java b9e59ef
flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 4bf36e6
flume-ng-doc/sphinx/FlumeUserGuide.rst 8eb3734
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java cf9724c
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java 1e642d8
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 5042d11
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 99ae010
Diff: https://reviews.apache.org/r/9427/diff/
Testing
-------
Yes
Thanks,
Ted Malaska