[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

classic Classic list List threaded Threaded
30 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
GitHub user phrocker opened a pull request:

    https://github.com/apache/accumulo/pull/140

    ACCUMULO-4419: Change how compression delegation works

    This commit includes A compression pool not based on CodecPool,
    a default factory that does no pooling. Changes are also
    in place that will allow us to better control some aspects of
    Compression within the tserver
   
    These are early changes, so I'm soliciting feedback before I continue. My main concerns are:
   
    Do we really need the ability to trim the codec pool and the other features that commons-pool provides.
    Does this really need to be configurable? I believe so as we are using this.
    What additional tests should I add? I'm working a few more but if there ideas I would gladly ignore them ;)
    Should I make anything else configurable for this first pass?
    When scrolling through the changes there are some comments I'd like to see so I'm going to update comments. If you have any input I'm all ears.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/phrocker/accumulo-1 ACCUMULO-4419

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/accumulo/pull/140.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #140
   
----
commit c3fd9984aa63dfd64b95b67955d9a967375cc464
Author: phrocker <[hidden email]>
Date:   2016-08-05T12:15:30Z

    ACCUMULO-4419: Change how compression delegation works
   
    This commit includes A compression pool not based on CodecPool,
    a default factory that does no pooling. Changes are also
    in place that will allow us to better control some aspects of
    Compression within the tserver

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75715696
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/conf/Property.java ---
    @@ -349,7 +349,18 @@
           "Memory to provide to batchwriter to replay mutations for replication"),
       TSERV_ASSIGNMENT_MAXCONCURRENT("tserver.assignment.concurrent.max", "2", PropertyType.COUNT,
           "The number of threads available to load tablets. Recoveries are still performed serially."),
    -
    --- End diff --
   
    More ideas for configuration please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75715831
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/codec/CompressionUpdater.java ---
    @@ -0,0 +1,84 @@
    +/*
    + * 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.accumulo.core.file.rfile.bcfile.codec;
    +
    +import jline.internal.Log;
    --- End diff --
   
    Remove.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75715837
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Compression.java ---
    @@ -420,7 +439,7 @@ public InputStream createDecompressionStream(InputStream downStream, Decompresso
             }
     
             CompressionInputStream cis = decomCodec.createInputStream(downStream, decompressor);
    -        BufferedInputStream bis2 = new BufferedInputStream(cis, DATA_IBUF_SIZE);
    +        BufferedInputStream bis2 = new BufferedInputStream(cis, dataInputBufferSize == 0 ? downStreamBufferSize : dataInputBufferSize);
    --- End diff --
   
    I'm going to move the conditional to separate lines to improve readability


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75718866
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/codec/CompressionUpdater.java ---
    @@ -0,0 +1,84 @@
    +/*
    + * 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.accumulo.core.file.rfile.bcfile.codec;
    +
    +import jline.internal.Log;
    --- End diff --
   
    oh noes! thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo issue #140: ACCUMULO-4419: Change how compression delegation works

joshelser
In reply to this post by joshelser
Github user phrocker commented on the issue:

    https://github.com/apache/accumulo/pull/140
 
    I didn't build the java doc. there are errors. I'm going to correct and re-submit. In the mean time please send along any comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75882156
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/codec/CompressorFactory.java ---
    @@ -0,0 +1,126 @@
    +/*
    + * 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.accumulo.core.file.rfile.bcfile.codec;
    +
    +import java.io.IOException;
    +
    +import org.apache.accumulo.core.conf.AccumuloConfiguration;
    +import org.apache.accumulo.core.file.rfile.bcfile.Compression.Algorithm;
    +import org.apache.hadoop.io.compress.Compressor;
    +import org.apache.hadoop.io.compress.Decompressor;
    +import org.apache.log4j.Logger;
    +
    +import com.google.common.base.Preconditions;
    +
    +/**
    + * Compressor Factory is a base class which creates compressors based on the supplied algorithm. Extensions may allow for alternative factory methods, such as
    + * object pooling.
    + */
    +public class CompressorFactory {
    --- End diff --
   
    Should this be an interface if it's a potential extension point?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75882856
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/codec/CompressorObjectFactory.java ---
    @@ -0,0 +1,54 @@
    +/*
    + * 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.accumulo.core.file.rfile.bcfile.codec;
    +
    +import org.apache.accumulo.core.file.rfile.bcfile.Compression.Algorithm;
    +import org.apache.commons.pool.KeyedPoolableObjectFactory;
    +import org.apache.hadoop.io.compress.Compressor;
    +
    +/**
    + * Factory pattern used to create compressors within CompressorPool
    + *
    + */
    +public class CompressorObjectFactory implements KeyedPoolableObjectFactory<Algorithm,Compressor> {
    --- End diff --
   
    Naming seems a little weird. "CompressorObjectFactory" and "CompressorFactory" seem to overlap in functionality based on the class name.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75883101
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/codec/CompressorPool.java ---
    @@ -0,0 +1,230 @@
    +/*
    + * 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.accumulo.core.file.rfile.bcfile.codec;
    +
    +import java.io.IOException;
    +
    +import org.apache.accumulo.core.conf.AccumuloConfiguration;
    +import org.apache.accumulo.core.conf.Property;
    +import org.apache.accumulo.core.file.rfile.bcfile.Compression.Algorithm;
    +import org.apache.commons.pool.impl.GenericKeyedObjectPool;
    +import org.apache.hadoop.io.compress.Compressor;
    +import org.apache.hadoop.io.compress.Decompressor;
    +import org.apache.log4j.Logger;
    +
    +import com.google.common.base.Preconditions;
    +
    +/**
    + * Compressor factory extension that enables object pooling using Commons Pool. The design will have a keyed compressor pool and decompressor pool. The key of
    + * which will be the Algorithm itself.
    + *
    + */
    +public class CompressorPool extends CompressorFactory {
    +
    +  private static final Logger LOG = Logger.getLogger(CompressorObjectFactory.class);
    +
    +  /**
    +   * Compressor pool.
    +   */
    +  GenericKeyedObjectPool<Algorithm,Compressor> compressorPool;
    +
    +  /**
    +   * Decompressor pool
    +   */
    +  GenericKeyedObjectPool<Algorithm,Decompressor> decompressorPool;
    +
    +  public CompressorPool(AccumuloConfiguration acuConf) {
    +
    +    super(acuConf);
    +
    +    compressorPool = new GenericKeyedObjectPool<Algorithm,Compressor>(new CompressorObjectFactory());
    +    // ensure that the pool grows when needed
    +    compressorPool.setWhenExhaustedAction(GenericKeyedObjectPool.WHEN_EXHAUSTED_GROW);
    +
    +    decompressorPool = new GenericKeyedObjectPool<Algorithm,Decompressor>(new DecompressorObjectFactory());
    +    // ensure that the pool grows when needed.
    +    decompressorPool.setWhenExhaustedAction(GenericKeyedObjectPool.WHEN_EXHAUSTED_GROW);
    +
    +    // perform the initial update.
    +    update(acuConf);
    +
    +  }
    +
    +  public void setMaxIdle(final int size) {
    --- End diff --
   
    Javadoc, please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75883337
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/codec/CompressorPool.java ---
    @@ -0,0 +1,230 @@
    +/*
    + * 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.accumulo.core.file.rfile.bcfile.codec;
    +
    +import java.io.IOException;
    +
    +import org.apache.accumulo.core.conf.AccumuloConfiguration;
    +import org.apache.accumulo.core.conf.Property;
    +import org.apache.accumulo.core.file.rfile.bcfile.Compression.Algorithm;
    +import org.apache.commons.pool.impl.GenericKeyedObjectPool;
    +import org.apache.hadoop.io.compress.Compressor;
    +import org.apache.hadoop.io.compress.Decompressor;
    +import org.apache.log4j.Logger;
    +
    +import com.google.common.base.Preconditions;
    +
    +/**
    + * Compressor factory extension that enables object pooling using Commons Pool. The design will have a keyed compressor pool and decompressor pool. The key of
    + * which will be the Algorithm itself.
    + *
    + */
    +public class CompressorPool extends CompressorFactory {
    +
    +  private static final Logger LOG = Logger.getLogger(CompressorObjectFactory.class);
    +
    +  /**
    +   * Compressor pool.
    +   */
    +  GenericKeyedObjectPool<Algorithm,Compressor> compressorPool;
    +
    +  /**
    +   * Decompressor pool
    +   */
    +  GenericKeyedObjectPool<Algorithm,Decompressor> decompressorPool;
    +
    +  public CompressorPool(AccumuloConfiguration acuConf) {
    +
    +    super(acuConf);
    +
    +    compressorPool = new GenericKeyedObjectPool<Algorithm,Compressor>(new CompressorObjectFactory());
    +    // ensure that the pool grows when needed
    +    compressorPool.setWhenExhaustedAction(GenericKeyedObjectPool.WHEN_EXHAUSTED_GROW);
    +
    +    decompressorPool = new GenericKeyedObjectPool<Algorithm,Decompressor>(new DecompressorObjectFactory());
    +    // ensure that the pool grows when needed.
    +    decompressorPool.setWhenExhaustedAction(GenericKeyedObjectPool.WHEN_EXHAUSTED_GROW);
    +
    +    // perform the initial update.
    +    update(acuConf);
    +
    +  }
    +
    +  public void setMaxIdle(final int size) {
    +    // check that we are changing the value.
    +    // this will avoid synchronization within the pool
    +    if (size != compressorPool.getMaxIdle())
    +      compressorPool.setMaxIdle(size);
    +    if (size != decompressorPool.getMaxIdle())
    +      decompressorPool.setMaxIdle(size);
    +  }
    +
    +  @Override
    +  public Compressor getCompressor(Algorithm compressionAlgorithm) throws IOException {
    +    Preconditions.checkNotNull(compressionAlgorithm, "Algorithm cannot be null");
    +    try {
    +      return compressorPool.borrowObject(compressionAlgorithm);
    +    } catch (Exception e) {
    --- End diff --
   
    Commons-Pool doesn't give us a named exception to differentiate between the Pool failing to construct a new instance and some pool operation failing? (your `catch Exception` clause)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75883491
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/codec/CompressorPool.java ---
    @@ -0,0 +1,230 @@
    +/*
    + * 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.accumulo.core.file.rfile.bcfile.codec;
    +
    +import java.io.IOException;
    +
    +import org.apache.accumulo.core.conf.AccumuloConfiguration;
    +import org.apache.accumulo.core.conf.Property;
    +import org.apache.accumulo.core.file.rfile.bcfile.Compression.Algorithm;
    +import org.apache.commons.pool.impl.GenericKeyedObjectPool;
    +import org.apache.hadoop.io.compress.Compressor;
    +import org.apache.hadoop.io.compress.Decompressor;
    +import org.apache.log4j.Logger;
    +
    +import com.google.common.base.Preconditions;
    +
    +/**
    + * Compressor factory extension that enables object pooling using Commons Pool. The design will have a keyed compressor pool and decompressor pool. The key of
    + * which will be the Algorithm itself.
    + *
    + */
    +public class CompressorPool extends CompressorFactory {
    +
    +  private static final Logger LOG = Logger.getLogger(CompressorObjectFactory.class);
    +
    +  /**
    +   * Compressor pool.
    +   */
    +  GenericKeyedObjectPool<Algorithm,Compressor> compressorPool;
    +
    +  /**
    +   * Decompressor pool
    +   */
    +  GenericKeyedObjectPool<Algorithm,Decompressor> decompressorPool;
    +
    +  public CompressorPool(AccumuloConfiguration acuConf) {
    +
    +    super(acuConf);
    +
    +    compressorPool = new GenericKeyedObjectPool<Algorithm,Compressor>(new CompressorObjectFactory());
    +    // ensure that the pool grows when needed
    +    compressorPool.setWhenExhaustedAction(GenericKeyedObjectPool.WHEN_EXHAUSTED_GROW);
    +
    +    decompressorPool = new GenericKeyedObjectPool<Algorithm,Decompressor>(new DecompressorObjectFactory());
    +    // ensure that the pool grows when needed.
    +    decompressorPool.setWhenExhaustedAction(GenericKeyedObjectPool.WHEN_EXHAUSTED_GROW);
    +
    +    // perform the initial update.
    +    update(acuConf);
    +
    +  }
    +
    +  public void setMaxIdle(final int size) {
    +    // check that we are changing the value.
    +    // this will avoid synchronization within the pool
    +    if (size != compressorPool.getMaxIdle())
    +      compressorPool.setMaxIdle(size);
    +    if (size != decompressorPool.getMaxIdle())
    +      decompressorPool.setMaxIdle(size);
    +  }
    +
    +  @Override
    +  public Compressor getCompressor(Algorithm compressionAlgorithm) throws IOException {
    +    Preconditions.checkNotNull(compressionAlgorithm, "Algorithm cannot be null");
    +    try {
    +      return compressorPool.borrowObject(compressionAlgorithm);
    +    } catch (Exception e) {
    +      // could not borrow the object, therefore we will attempt to create it
    +      // this will most likely result in an exception when returning so an end will occur
    +      LOG.warn("Could not borrow compressor; creating instead", e);
    +      return compressionAlgorithm.getCodec().createCompressor();
    +    }
    +  }
    +
    +  @Override
    +  public void releaseCompressor(Algorithm compressionAlgorithm, Compressor compressor) {
    +    Preconditions.checkNotNull(compressionAlgorithm, "Algorithm cannot be null");
    +    Preconditions.checkNotNull(compressor, "Compressor should not be null");
    +    try {
    +      compressorPool.returnObject(compressionAlgorithm, compressor);
    +    } catch (Exception e) {
    +      LOG.warn("Could not return compressor; closing instead", e);
    +      // compressor failed to be returned. Let's free the memory associated with it
    +      compressor.end();
    +    }
    +
    +  }
    +
    +  @Override
    +  public void releaseDecompressor(Algorithm compressionAlgorithm, Decompressor decompressor) {
    +    Preconditions.checkNotNull(compressionAlgorithm, "Algorithm cannot be null");
    +    Preconditions.checkNotNull(decompressor, "Deompressor should not be null");
    +    try {
    +      decompressorPool.returnObject(compressionAlgorithm, decompressor);
    +    } catch (Exception e) {
    +      LOG.warn("Could not return decompressor; closing instead", e);
    +      // compressor failed to be returned. Let's free the memory associated with it
    +      decompressor.end();
    +    }
    +
    +  }
    +
    +  @Override
    +  public Decompressor getDecompressor(Algorithm compressionAlgorithm) {
    +    Preconditions.checkNotNull(compressionAlgorithm, "Algorithm cannot be null");
    +    try {
    +      return decompressorPool.borrowObject(compressionAlgorithm);
    +    } catch (Exception e) {
    +      LOG.warn("Could not borrow decompressor; creating instead", e);
    +      // could not borrow the object, therefore we will attempt to create it
    +      // this will most likely result in an exception when returning so an end will occur
    +      return compressionAlgorithm.getCodec().createDecompressor();
    +    }
    +  }
    +
    +  /**
    +   * Closes both pools, which will clear and evict the respective compressor/decompressors. {@inheritDoc}
    +   */
    +  @Override
    +  public void close() {
    +    try {
    +      compressorPool.close();
    +    } catch (Exception e) {
    +      LOG.error(e);
    --- End diff --
   
    An associated error message would be good.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75884424
 
    --- Diff: core/src/test/java/org/apache/accumulo/core/file/rfile/bcfile/codec/CompressorFactoryTest.java ---
    @@ -0,0 +1,172 @@
    +/*
    + * 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.accumulo.core.file.rfile.bcfile.codec;
    +
    +import static org.junit.Assert.fail;
    +
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.Set;
    +
    +import org.apache.accumulo.core.conf.AccumuloConfiguration;
    +import org.apache.accumulo.core.file.rfile.bcfile.Compression;
    +import org.apache.accumulo.core.file.rfile.bcfile.Compression.Algorithm;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.io.compress.CompressionCodec;
    +import org.apache.hadoop.io.compress.Compressor;
    +import org.apache.hadoop.io.compress.Decompressor;
    +import org.apache.hadoop.util.ReflectionUtils;
    +import org.junit.Assert;
    +import org.junit.Before;
    +import org.junit.Test;
    +
    +public class CompressorFactoryTest {
    +
    +  HashMap<Compression.Algorithm,Boolean> isSupported = new HashMap<Compression.Algorithm,Boolean>();
    +
    +  @Before
    +  public void testSupport() {
    +    // we can safely assert that GZ exists by virtue of it being the DefaultCodec
    +    isSupported.put(Compression.Algorithm.GZ, true);
    +
    +    Configuration myConf = new Configuration();
    +
    +    String extClazz = System.getProperty(Compression.Algorithm.CONF_LZO_CLASS);
    +    String clazz = (extClazz != null) ? extClazz : "org.apache.hadoop.io.compress.LzoCodec";
    +    try {
    +      CompressionCodec codec = (CompressionCodec) ReflectionUtils.newInstance(Class.forName(clazz), myConf);
    +
    +      Assert.assertNotNull(codec);
    +      isSupported.put(Compression.Algorithm.LZO, true);
    +
    +    } catch (ClassNotFoundException e) {
    +      // that is okay
    +    }
    +
    +  }
    +
    +  @Test
    +  public void testAlgoreithms() throws IOException {
    +    CompressorFactory factory = new CompressorFactory(AccumuloConfiguration.getDefaultConfiguration());
    +    for (final Algorithm al : Algorithm.values()) {
    +      if (isSupported.get(al) != null && isSupported.get(al) == true) {
    +
    +        Compressor compressor = factory.getCompressor(al);
    +        Assert.assertNotNull(compressor);
    +        factory.releaseCompressor(al, compressor);
    +
    +        Decompressor decompressor = factory.getDecompressor(al);
    +        Assert.assertNotNull(decompressor);
    +        factory.releaseDecompressor(al, decompressor);
    +      }
    +    }
    +  }
    +
    +  @Test
    +  public void testMultipleNotTheSameCompressors() throws IOException {
    +    CompressorFactory factory = new CompressorFactory(AccumuloConfiguration.getDefaultConfiguration());
    +    for (final Algorithm al : Algorithm.values()) {
    +      if (isSupported.get(al) != null && isSupported.get(al) == true) {
    +
    +        Set<Integer> compressorHashCodes = new HashSet<>();
    +        ArrayList<Compressor> compressors = new ArrayList<>();
    +        for (int i = 0; i < 25; i++) {
    +          Compressor compressor = factory.getCompressor(al);
    +          Assert.assertNotNull(compressor);
    +          compressors.add(compressor);
    +          compressorHashCodes.add(Integer.valueOf(System.identityHashCode(compressor)));
    +        }
    +
    +        // assert that we have 25 with this particular factory.
    +        Assert.assertEquals(25, compressorHashCodes.size());
    +
    +        // free them for posterity sake
    +        for (Compressor compressor : compressors) {
    +          factory.releaseCompressor(al, compressor);
    +        }
    +      }
    +    }
    +
    +  }
    +
    +  @Test
    +  public void testMultipleNotTheSameDeompressors() throws IOException {
    --- End diff --
   
    nit, spelling


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75884568
 
    --- Diff: core/src/test/java/org/apache/accumulo/core/file/rfile/bcfile/codec/CompressorFactoryTest.java ---
    @@ -0,0 +1,172 @@
    +/*
    + * 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.accumulo.core.file.rfile.bcfile.codec;
    +
    +import static org.junit.Assert.fail;
    +
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.Set;
    +
    +import org.apache.accumulo.core.conf.AccumuloConfiguration;
    +import org.apache.accumulo.core.file.rfile.bcfile.Compression;
    +import org.apache.accumulo.core.file.rfile.bcfile.Compression.Algorithm;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.io.compress.CompressionCodec;
    +import org.apache.hadoop.io.compress.Compressor;
    +import org.apache.hadoop.io.compress.Decompressor;
    +import org.apache.hadoop.util.ReflectionUtils;
    +import org.junit.Assert;
    +import org.junit.Before;
    +import org.junit.Test;
    +
    +public class CompressorFactoryTest {
    +
    +  HashMap<Compression.Algorithm,Boolean> isSupported = new HashMap<Compression.Algorithm,Boolean>();
    +
    +  @Before
    +  public void testSupport() {
    +    // we can safely assert that GZ exists by virtue of it being the DefaultCodec
    +    isSupported.put(Compression.Algorithm.GZ, true);
    +
    +    Configuration myConf = new Configuration();
    +
    +    String extClazz = System.getProperty(Compression.Algorithm.CONF_LZO_CLASS);
    +    String clazz = (extClazz != null) ? extClazz : "org.apache.hadoop.io.compress.LzoCodec";
    +    try {
    +      CompressionCodec codec = (CompressionCodec) ReflectionUtils.newInstance(Class.forName(clazz), myConf);
    +
    +      Assert.assertNotNull(codec);
    +      isSupported.put(Compression.Algorithm.LZO, true);
    +
    +    } catch (ClassNotFoundException e) {
    +      // that is okay
    +    }
    +
    +  }
    +
    +  @Test
    +  public void testAlgoreithms() throws IOException {
    +    CompressorFactory factory = new CompressorFactory(AccumuloConfiguration.getDefaultConfiguration());
    +    for (final Algorithm al : Algorithm.values()) {
    +      if (isSupported.get(al) != null && isSupported.get(al) == true) {
    --- End diff --
   
    Could lift the null && true check into a method for clarity since it's copied in a couple of places.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75884825
 
    --- Diff: pom.xml ---
    @@ -223,6 +223,11 @@
             <version>1.1.1</version>
           </dependency>
           <dependency>
    +        <groupId>commons-pool</groupId>
    +        <artifactId>commons-pool</artifactId>
    +        <version>1.6</version>
    --- End diff --
   
    Should we be trying to use org.apache.commons:commons-pool2 instead? I'm not sure if commons-pool is still receiving updates.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75886596
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Compression.java ---
    @@ -497,69 +513,66 @@ public CompressionCodec load(Entry<Algorithm,Integer> key) {
         public abstract boolean isSupported();
     
         public Compressor getCompressor() throws IOException {
    -      CompressionCodec codec = getCodec();
    -      if (codec != null) {
    -        Compressor compressor = CodecPool.getCompressor(codec);
    -        if (compressor != null) {
    -          if (compressor.finished()) {
    -            // Somebody returns the compressor to CodecPool but is still using
    -            // it.
    -            LOG.warn("Compressor obtained from CodecPool already finished()");
    -          } else {
    -            LOG.debug("Got a compressor: " + compressor.hashCode());
    -          }
    -          /**
    -           * Following statement is necessary to get around bugs in 0.18 where a compressor is referenced after returned back to the codec pool.
    -           */
    -          compressor.reset();
    -        }
    -        return compressor;
    -      }
    -      return null;
    +      return compressorFactory.getCompressor(this);
         }
     
         public void returnCompressor(Compressor compressor) {
    -      if (compressor != null) {
    -        LOG.debug("Return a compressor: " + compressor.hashCode());
    -        CodecPool.returnCompressor(compressor);
    -      }
    +      compressorFactory.releaseCompressor(this, compressor);
         }
     
         public Decompressor getDecompressor() throws IOException {
    -      CompressionCodec codec = getCodec();
    -      if (codec != null) {
    -        Decompressor decompressor = CodecPool.getDecompressor(codec);
    -        if (decompressor != null) {
    -          if (decompressor.finished()) {
    -            // Somebody returns the decompressor to CodecPool but is still using
    -            // it.
    -            LOG.warn("Decompressor obtained from CodecPool already finished()");
    -          } else {
    -            LOG.debug("Got a decompressor: " + decompressor.hashCode());
    -          }
    -          /**
    -           * Following statement is necessary to get around bugs in 0.18 where a decompressor is referenced after returned back to the codec pool.
    -           */
    -          decompressor.reset();
    -        }
    -        return decompressor;
    -      }
    -
    -      return null;
    +      return compressorFactory.getDecompressor(this);
         }
     
         public void returnDecompressor(Decompressor decompressor) {
    -      if (decompressor != null) {
    -        LOG.debug("Returned a decompressor: " + decompressor.hashCode());
    -        CodecPool.returnDecompressor(decompressor);
    -      }
    +      compressorFactory.releaseDecompressor(this, decompressor);
         }
     
         public String getName() {
           return compressName;
         }
       }
     
    +  /**
    +   * Default implementation will create new compressors.
    +   */
    +  private static CompressorFactory compressorFactory = new CompressorFactory(null);
    --- End diff --
   
    I think making this a `final AtomicReference<CompressorFactory>` would be good (corresponding comment below).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r75886697
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Compression.java ---
    @@ -497,69 +513,66 @@ public CompressionCodec load(Entry<Algorithm,Integer> key) {
         public abstract boolean isSupported();
     
         public Compressor getCompressor() throws IOException {
    -      CompressionCodec codec = getCodec();
    -      if (codec != null) {
    -        Compressor compressor = CodecPool.getCompressor(codec);
    -        if (compressor != null) {
    -          if (compressor.finished()) {
    -            // Somebody returns the compressor to CodecPool but is still using
    -            // it.
    -            LOG.warn("Compressor obtained from CodecPool already finished()");
    -          } else {
    -            LOG.debug("Got a compressor: " + compressor.hashCode());
    -          }
    -          /**
    -           * Following statement is necessary to get around bugs in 0.18 where a compressor is referenced after returned back to the codec pool.
    -           */
    -          compressor.reset();
    -        }
    -        return compressor;
    -      }
    -      return null;
    +      return compressorFactory.getCompressor(this);
         }
     
         public void returnCompressor(Compressor compressor) {
    -      if (compressor != null) {
    -        LOG.debug("Return a compressor: " + compressor.hashCode());
    -        CodecPool.returnCompressor(compressor);
    -      }
    +      compressorFactory.releaseCompressor(this, compressor);
         }
     
         public Decompressor getDecompressor() throws IOException {
    -      CompressionCodec codec = getCodec();
    -      if (codec != null) {
    -        Decompressor decompressor = CodecPool.getDecompressor(codec);
    -        if (decompressor != null) {
    -          if (decompressor.finished()) {
    -            // Somebody returns the decompressor to CodecPool but is still using
    -            // it.
    -            LOG.warn("Decompressor obtained from CodecPool already finished()");
    -          } else {
    -            LOG.debug("Got a decompressor: " + decompressor.hashCode());
    -          }
    -          /**
    -           * Following statement is necessary to get around bugs in 0.18 where a decompressor is referenced after returned back to the codec pool.
    -           */
    -          decompressor.reset();
    -        }
    -        return decompressor;
    -      }
    -
    -      return null;
    +      return compressorFactory.getDecompressor(this);
         }
     
         public void returnDecompressor(Decompressor decompressor) {
    -      if (decompressor != null) {
    -        LOG.debug("Returned a decompressor: " + decompressor.hashCode());
    -        CodecPool.returnDecompressor(decompressor);
    -      }
    +      compressorFactory.releaseDecompressor(this, decompressor);
         }
     
         public String getName() {
           return compressName;
         }
       }
     
    +  /**
    +   * Default implementation will create new compressors.
    +   */
    +  private static CompressorFactory compressorFactory = new CompressorFactory(null);
    +
    +  /**
    +   * Allow the compressor factory to be set within this Instance.
    +   *
    +   * @param compFactory
    +   *          incoming compressor factory to be used by all Algorithms
    +   */
    +  public static synchronized void setCompressionFactory(final CompressorFactory compFactory) {
    --- End diff --
   
    With the `AtomicReference`, we could remove the synchronization and just close the previous value we get from the `getAndSet(V)` method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo issue #140: ACCUMULO-4419: Change how compression delegation works

joshelser
In reply to this post by joshelser
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/140
 
    On overall comment: let's use slf4j loggers instead of log4j specifically. I see you have log4j classes sprinkled around.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user ctubbsii commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r76876717
 
    --- Diff: pom.xml ---
    @@ -223,6 +223,11 @@
             <version>1.1.1</version>
           </dependency>
           <dependency>
    +        <groupId>commons-pool</groupId>
    +        <artifactId>commons-pool</artifactId>
    +        <version>1.6</version>
    --- End diff --
   
    Yes, I think we should. For a new dependency, I think we should be using the latest available, to ensure the best upstream support.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r76880275
 
    --- Diff: pom.xml ---
    @@ -223,6 +223,11 @@
             <version>1.1.1</version>
           </dependency>
           <dependency>
    +        <groupId>commons-pool</groupId>
    +        <artifactId>commons-pool</artifactId>
    +        <version>1.6</version>
    --- End diff --
   
    Agreed. Not sure why I chose 1.6  when I have been using Commons pool2


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #140: ACCUMULO-4419: Change how compression delegation...

joshelser
In reply to this post by joshelser
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/140#discussion_r76880451
 
    --- Diff: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Compression.java ---
    @@ -497,69 +513,66 @@ public CompressionCodec load(Entry<Algorithm,Integer> key) {
         public abstract boolean isSupported();
     
         public Compressor getCompressor() throws IOException {
    -      CompressionCodec codec = getCodec();
    -      if (codec != null) {
    -        Compressor compressor = CodecPool.getCompressor(codec);
    -        if (compressor != null) {
    -          if (compressor.finished()) {
    -            // Somebody returns the compressor to CodecPool but is still using
    -            // it.
    -            LOG.warn("Compressor obtained from CodecPool already finished()");
    -          } else {
    -            LOG.debug("Got a compressor: " + compressor.hashCode());
    -          }
    -          /**
    -           * Following statement is necessary to get around bugs in 0.18 where a compressor is referenced after returned back to the codec pool.
    -           */
    -          compressor.reset();
    -        }
    -        return compressor;
    -      }
    -      return null;
    +      return compressorFactory.getCompressor(this);
         }
     
         public void returnCompressor(Compressor compressor) {
    -      if (compressor != null) {
    -        LOG.debug("Return a compressor: " + compressor.hashCode());
    -        CodecPool.returnCompressor(compressor);
    -      }
    +      compressorFactory.releaseCompressor(this, compressor);
         }
     
         public Decompressor getDecompressor() throws IOException {
    -      CompressionCodec codec = getCodec();
    -      if (codec != null) {
    -        Decompressor decompressor = CodecPool.getDecompressor(codec);
    -        if (decompressor != null) {
    -          if (decompressor.finished()) {
    -            // Somebody returns the decompressor to CodecPool but is still using
    -            // it.
    -            LOG.warn("Decompressor obtained from CodecPool already finished()");
    -          } else {
    -            LOG.debug("Got a decompressor: " + decompressor.hashCode());
    -          }
    -          /**
    -           * Following statement is necessary to get around bugs in 0.18 where a decompressor is referenced after returned back to the codec pool.
    -           */
    -          decompressor.reset();
    -        }
    -        return decompressor;
    -      }
    -
    -      return null;
    +      return compressorFactory.getDecompressor(this);
         }
     
         public void returnDecompressor(Decompressor decompressor) {
    -      if (decompressor != null) {
    -        LOG.debug("Returned a decompressor: " + decompressor.hashCode());
    -        CodecPool.returnDecompressor(decompressor);
    -      }
    +      compressorFactory.releaseDecompressor(this, decompressor);
         }
     
         public String getName() {
           return compressName;
         }
       }
     
    +  /**
    +   * Default implementation will create new compressors.
    +   */
    +  private static CompressorFactory compressorFactory = new CompressorFactory(null);
    --- End diff --
   
    Ahh yah.... Good call.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
12
Loading...