[
https://issues.apache.org/jira/browse/HADOOP-5879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12732943#action_12732943
]
Chris Douglas commented on HADOOP-5879:
---------------------------------------
Thanks for updating the patch
* Rather than having special semantics for construct, I'd suggest removing the
directBufferSize formal from construct and returning the allocation to the
constructor
* Sorry, my last comment was not clear. By "implementations should also
describe..." I meant that the classes implementing reinit should include a
description of what it effects in its javadoc for reinit, as you did in
BuiltInZlibDeflater. I didn't mean that more logging was required.
Compressor::reinit should also describe the contract for future implementers,
"Prepare the compressor to be used in a new stream with settings defined in the
given Configuration" or something like that
* I think it's appropriate to fail if the configuration is invalid rather than
taking the default in ZlibFactory::getCompression\* (why ZlibFactory?). I don't
think the getDefault\* methods are necessary. setCompression\* should take the
appropriate enum, rather than String. Filed HADOOP-6161 for get/setEnum to
simplify some of the conf-related code
* In ZlibCompressor::reinit, reset is not called if the Configuration is null.
For users calling these methods not via CodecPool, reset() should probably be
called
* CompressionLevel::compressionLevel() and
CompressionStrategy::compressionStrategy() should remain package-private; the
integers are implementation details
* The unit test doesn't really verify the functionality added by this patch,
save that it doesn't throw exceptions. That said, it would be hard to verify
that this is working as expected without adding get\* methods to
ZlibCompressor. Can you describe how you verified the patch's correctness, both
for the native and non-native codecs?
> GzipCodec should read compression level etc from configuration
> --------------------------------------------------------------
>
> Key: HADOOP-5879
> URL: https://issues.apache.org/jira/browse/HADOOP-5879
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Reporter: Zheng Shao
> Assignee: He Yongqiang
> Attachments: hadoop-5879-5-21.patch, hadoop-5879-7-13-2.patch,
> hadoop-5879-7-13-3.patch, hadoop-5879-7-18-3.patch
>
>
> GzipCodec currently uses the default compression level. We should allow
> overriding the default value from Configuration.
> {code}
> static final class GzipZlibCompressor extends ZlibCompressor {
> public GzipZlibCompressor() {
> super(ZlibCompressor.CompressionLevel.DEFAULT_COMPRESSION,
> ZlibCompressor.CompressionStrategy.DEFAULT_STRATEGY,
> ZlibCompressor.CompressionHeader.GZIP_FORMAT, 64*1024);
> }
> }
> {code}
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
