accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From keith-turner <...@git.apache.org>
Subject [GitHub] accumulo pull request #106: ACCUMULO-4153: Update the getCodec method to no ...
Date Fri, 03 Jun 2016 15:13:54 GMT
Github user keith-turner commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/106#discussion_r65724278
  
    --- Diff: core/src/test/java/org/apache/accumulo/core/file/rfile/bcfile/CompressionTest.java
---
    @@ -18,109 +18,233 @@
     
     import java.io.IOException;
     import java.util.ArrayList;
    +import java.util.HashMap;
    +import java.util.HashSet;
     import java.util.concurrent.Callable;
    +import java.util.concurrent.ExecutionException;
     import java.util.concurrent.ExecutorService;
     import java.util.concurrent.Executors;
    -import java.util.HashSet;
    +import java.util.concurrent.Future;
    +import java.util.concurrent.TimeUnit;
     
     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.util.ReflectionUtils;
     import org.junit.Assert;
    +import org.junit.Before;
     import org.junit.Test;
     
     import com.google.common.collect.Lists;
    +import com.google.common.collect.Maps;
     import com.google.common.collect.Sets;
     
     public class CompressionTest {
     
    +  HashMap<Compression.Algorithm,Boolean> isSupported = Maps.newHashMap();
    +
    +  @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
    +    }
    +
    +    extClazz = System.getProperty(Compression.Algorithm.CONF_SNAPPY_CLASS);
    +    clazz = (extClazz != null) ? extClazz : "org.apache.hadoop.io.compress.SnappyCodec";
    +    try {
    +      CompressionCodec codec = (CompressionCodec) ReflectionUtils.newInstance(Class.forName(clazz),
myConf);
    +
    +      Assert.assertNotNull(codec);
    +
    +      isSupported.put(Compression.Algorithm.SNAPPY, true);
    +
    +    } catch (ClassNotFoundException e) {
    +      // that is okay
    +    }
    +
    +  }
    +
       @Test
       public void testSingle() throws IOException {
    -    Assert.assertNotNull(Compression.Algorithm.GZ.getCodec());
     
    -    Assert.assertNotNull(Compression.Algorithm.GZ.getCodec());
    +    for (final Algorithm al : Algorithm.values()) {
    +      if (isSupported.get(al) != null && isSupported.get(al) == true) {
    +
    +        // first call to issupported should be true
    +        Assert.assertTrue(al + " is not supported, but should be", al.isSupported());
    +
    +        Assert.assertNotNull(al + " should have a non-null codec", al.getCodec());
    +
    +        Assert.assertNotNull(al + " should have a non-null codec", al.getCodec());
    +      }
    +    }
       }
     
       @Test
    -  public void testManyStartNotNull() throws IOException {
    -    final CompressionCodec codec = Algorithm.GZ.getCodec();
    +  public void testSingleNoSideEffect() throws IOException {
    +
    +    for (final Algorithm al : Algorithm.values()) {
    +      if (isSupported.get(al) != null && isSupported.get(al) == true) {
    +
    +        Assert.assertTrue(al + " is not supported, but should be", al.isSupported());
    +
    +        Assert.assertNotNull(al + " should have a non-null codec", al.getCodec());
    +
    +        // assert that additional calls to create will not create
    +        // additional codecs
    +
    +        Assert.assertNotEquals(al + " should have created a new codec, but did not",
System.identityHashCode(al.getCodec()), al.createNewCodec(88 * 1024));
    +      }
    +    }
    +  }
    +
    +  @Test(timeout = 60 * 1000)
    +  public void testManyStartNotNull() throws IOException, InterruptedException, ExecutionException
{
    +
    +    for (final Algorithm al : Algorithm.values()) {
    +      if (isSupported.get(al) != null && isSupported.get(al) == true) {
     
    -    Assert.assertNotNull(codec);
    +        // first call to issupported should be true
    +        Assert.assertTrue(al + " is not supported, but should be", al.isSupported());
     
    -    ExecutorService service = Executors.newFixedThreadPool(10);
    +        final CompressionCodec codec = al.getCodec();
     
    -    for (int i = 0; i < 30; i++) {
    -      service.submit(new Callable<Boolean>()
    +        Assert.assertNotNull(al + " should not be null", codec);
     
    -      {
    +        ExecutorService service = Executors.newFixedThreadPool(10);
     
    -        @Override
    -        public Boolean call() throws Exception {
    -          Assert.assertNotNull(Compression.Algorithm.GZ.getCodec());
    -          return true;
    +        ArrayList<Future<Boolean>> results = Lists.newArrayList();
    +
    +        for (int i = 0; i < 30; i++) {
    +          results.add(service.submit(new Callable<Boolean>()
    +
    +          {
    +
    +            @Override
    +            public Boolean call() throws Exception {
    +              Assert.assertNotNull(al + " should not be null", al.getCodec());
    +              return true;
    +            }
    +
    +          }));
             }
     
    -      });
    -    }
    +        service.shutdown();
     
    -    service.shutdown();
    +        Assert.assertNotNull(al + " should not be null", codec);
     
    -    Assert.assertNotNull(codec);
    +        while (!service.awaitTermination(1, TimeUnit.SECONDS)) {
    +          // wait
    +        }
    +
    +        for (Future<Boolean> result : results) {
    +          Assert.assertTrue(al + " resulted in a failed call to getcodec within the thread
pool", result.get());
    +        }
    +      }
    +    }
     
       }
     
       // don't start until we have created the codec
    -  @Test
    -  public void testManyDontStartUntilThread() throws IOException {
    +  @Test(timeout = 60 * 1000)
    +  public void testManyDontStartUntilThread() throws IOException, InterruptedException,
ExecutionException {
    +
    +    for (final Algorithm al : Algorithm.values()) {
    +      if (isSupported.get(al) != null && isSupported.get(al) == true) {
    +
    +        // first call to issupported should be true
    +        Assert.assertTrue(al + " is not supported, but should be", al.isSupported());
     
    -    ExecutorService service = Executors.newFixedThreadPool(10);
    +        ExecutorService service = Executors.newFixedThreadPool(10);
     
    -    for (int i = 0; i < 30; i++) {
    +        ArrayList<Future<Boolean>> results = Lists.newArrayList();
     
    -      service.submit(new Callable<Boolean>() {
    +        for (int i = 0; i < 30; i++) {
     
    -        @Override
    -        public Boolean call() throws Exception {
    -          Assert.assertNotNull(Compression.Algorithm.GZ.getCodec());
    -          return true;
    +          results.add(service.submit(new Callable<Boolean>() {
    +
    +            @Override
    +            public Boolean call() throws Exception {
    +              Assert.assertNotNull(al + " should have a non-null codec", al.getCodec());
    +              return true;
    +            }
    +
    +          }));
             }
     
    -      });
    -    }
    +        service.shutdown();
     
    -    service.shutdown();
    +        while (!service.awaitTermination(1, TimeUnit.SECONDS)) {
    +          // wait
    +        }
    +
    +        for (Future<Boolean> result : results) {
    +          Assert.assertTrue(al + " resulted in a failed call to getcodec within the thread
pool", result.get());
    +        }
    +      }
    +    }
     
       }
     
    -  // don't start until we have created the codec
    -  @Test
    -  public void testThereCanBeOnlyOne() throws IOException, InterruptedException {
    +  @Test(timeout = 60 * 1000)
    +  public void testThereCanBeOnlyOne() throws IOException, InterruptedException, ExecutionException
{
    --- End diff --
    
    Would an additional test like the following make sense?
    * before threads call `createNewCodec(42)`  to create and cache a codec with that buffer
size.. verify it differs from default
    * have a bunch of threads call  `createNewCodec(42)` ... verify the get same codec



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message