From 1e7fd0c23c3477e48901eea2a4d1ef61d5eb1519 Mon Sep 17 00:00:00 2001 From: Owen O'Malley Date: Tue, 24 May 2011 18:48:37 +0000 Subject: [PATCH] HADOOP-7258. The Gzip codec should not return null decompressors. (omalley) git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1127215 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 6 +++ .../apache/hadoop/io/compress/CodecPool.java | 8 +++ .../apache/hadoop/io/compress/DoNotPool.java | 35 +++++++++++++ .../zlib/BuiltInGzipDecompressor.java | 2 + .../apache/hadoop/io/compress/TestCodec.java | 52 ++++++++++++++++++- 5 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 src/java/org/apache/hadoop/io/compress/DoNotPool.java diff --git a/CHANGES.txt b/CHANGES.txt index d75a6aa6381..a753acecf05 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -2388,6 +2388,12 @@ Release 0.20.3 - Unreleased HADOOP-7072. Remove java5 dependencies from build. (cos) +Release 0.20.203.0 - 2011-5-11 + + BUG FIXES + + HADOOP-7258. The Gzip codec should not return null decompressors. (omalley) + Release 0.20.2 - 2010-2-16 NEW FEATURES diff --git a/src/java/org/apache/hadoop/io/compress/CodecPool.java b/src/java/org/apache/hadoop/io/compress/CodecPool.java index d878f62ebf7..ed76012f808 100644 --- a/src/java/org/apache/hadoop/io/compress/CodecPool.java +++ b/src/java/org/apache/hadoop/io/compress/CodecPool.java @@ -149,6 +149,10 @@ public class CodecPool { if (compressor == null) { return; } + // if the compressor can't be reused, don't pool it. + if (compressor.getClass().isAnnotationPresent(DoNotPool.class)) { + return; + } compressor.reset(); payback(compressorPool, compressor); } @@ -163,6 +167,10 @@ public class CodecPool { if (decompressor == null) { return; } + // if the decompressor can't be reused, don't pool it. + if (decompressor.getClass().isAnnotationPresent(DoNotPool.class)) { + return; + } decompressor.reset(); payback(decompressorPool, decompressor); } diff --git a/src/java/org/apache/hadoop/io/compress/DoNotPool.java b/src/java/org/apache/hadoop/io/compress/DoNotPool.java new file mode 100644 index 00000000000..c701b87c310 --- /dev/null +++ b/src/java/org/apache/hadoop/io/compress/DoNotPool.java @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.io.compress; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * This is a marker annotation that marks a compressor or decompressor + * type as not to be pooled. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +@Documented +public @interface DoNotPool { + +} \ No newline at end of file diff --git a/src/java/org/apache/hadoop/io/compress/zlib/BuiltInGzipDecompressor.java b/src/java/org/apache/hadoop/io/compress/zlib/BuiltInGzipDecompressor.java index 0905f122c54..1e5525e743b 100644 --- a/src/java/org/apache/hadoop/io/compress/zlib/BuiltInGzipDecompressor.java +++ b/src/java/org/apache/hadoop/io/compress/zlib/BuiltInGzipDecompressor.java @@ -24,12 +24,14 @@ import java.util.zip.Inflater; import org.apache.hadoop.util.PureJavaCrc32; import org.apache.hadoop.io.compress.Decompressor; +import org.apache.hadoop.io.compress.DoNotPool; /** * A {@link Decompressor} based on the popular gzip compressed file format. * http://www.gzip.org/ * */ +@DoNotPool public class BuiltInGzipDecompressor implements Decompressor { private static final int GZIP_MAGIC_ID = 0x8b1f; // if read as LE short int private static final int GZIP_DEFLATE_METHOD = 8; diff --git a/src/test/core/org/apache/hadoop/io/compress/TestCodec.java b/src/test/core/org/apache/hadoop/io/compress/TestCodec.java index 26981d312f1..8981308cb3d 100644 --- a/src/test/core/org/apache/hadoop/io/compress/TestCodec.java +++ b/src/test/core/org/apache/hadoop/io/compress/TestCodec.java @@ -675,7 +675,8 @@ public class TestCodec { // Create a GZIP text file via the Compressor interface. CompressionCodecFactory ccf = new CompressionCodecFactory(conf); CompressionCodec codec = ccf.getCodec(new Path("foo.gz")); - assertTrue("Codec for .gz file is not GzipCodec", codec instanceof GzipCodec); + assertTrue("Codec for .gz file is not GzipCodec", + codec instanceof GzipCodec); final String msg = "This is the message we are going to compress."; final String tmpDir = System.getProperty("test.build.data", "/tmp/"); @@ -715,4 +716,53 @@ public class TestCodec { public void testGzipNativeCodecWrite() throws IOException { testGzipCodecWrite(true); } + + public void testCodecPoolAndGzipDecompressor() { + // BuiltInZlibInflater should not be used as the GzipCodec decompressor. + // Assert that this is the case. + + // Don't use native libs for this test. + Configuration conf = new Configuration(); + conf.setBoolean("hadoop.native.lib", false); + assertFalse("ZlibFactory is using native libs against request", + ZlibFactory.isNativeZlibLoaded(conf)); + + // This should give us a BuiltInZlibInflater. + Decompressor zlibDecompressor = ZlibFactory.getZlibDecompressor(conf); + assertNotNull("zlibDecompressor is null!", zlibDecompressor); + assertTrue("ZlibFactory returned unexpected inflator", + zlibDecompressor instanceof BuiltInZlibInflater); + // its createOutputStream() just wraps the existing stream in a + // java.util.zip.GZIPOutputStream. + CompressionCodecFactory ccf = new CompressionCodecFactory(conf); + CompressionCodec codec = ccf.getCodec(new Path("foo.gz")); + assertTrue("Codec for .gz file is not GzipCodec", + codec instanceof GzipCodec); + + // make sure we don't get a null decompressor + Decompressor codecDecompressor = codec.createDecompressor(); + if (null == codecDecompressor) { + fail("Got null codecDecompressor"); + } + + // Asking the CodecPool for a decompressor for GzipCodec + // should not return null + Decompressor poolDecompressor = CodecPool.getDecompressor(codec); + if (null == poolDecompressor) { + fail("Got null poolDecompressor"); + } + // return a couple decompressors + CodecPool.returnDecompressor(zlibDecompressor); + CodecPool.returnDecompressor(poolDecompressor); + Decompressor poolDecompressor2 = CodecPool.getDecompressor(codec); + if (poolDecompressor.getClass() == BuiltInGzipDecompressor.class) { + if (poolDecompressor == poolDecompressor2) { + fail("Reused java gzip decompressor in pool"); + } + } else { + if (poolDecompressor != poolDecompressor2) { + fail("Did not reuse native gzip decompressor in pool"); + } + } + } }