From c31f5ce498bd29585b9d1e9566f202b179db4850 Mon Sep 17 00:00:00 2001 From: leventov Date: Tue, 4 Oct 2016 14:27:06 +0300 Subject: [PATCH 1/4] Address some HuntBugs warnings --- src/main/java/com/metamx/common/CompressionUtils.java | 2 +- src/main/java/com/metamx/common/Granularity.java | 3 --- src/main/java/com/metamx/common/RetryUtils.java | 3 ++- src/main/java/com/metamx/common/StreamUtils.java | 4 ++-- src/main/java/com/metamx/common/StringUtils.java | 1 - src/main/java/com/metamx/common/guava/Comparators.java | 2 +- .../metamx/common/guava/LimitedYieldingAccumulator.java | 1 + src/main/java/com/metamx/common/parsers/CSVParser.java | 1 - .../java/com/metamx/common/parsers/DelimitedParser.java | 1 - .../java/com/metamx/common/parsers/JSONPathParser.java | 7 +++---- .../java/com/metamx/common/parsers/TimestampParser.java | 1 - 11 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/metamx/common/CompressionUtils.java b/src/main/java/com/metamx/common/CompressionUtils.java index 8f3857e9..4cdc7923 100644 --- a/src/main/java/com/metamx/common/CompressionUtils.java +++ b/src/main/java/com/metamx/common/CompressionUtils.java @@ -148,7 +148,7 @@ public FileUtils.FileCopyResult call() throws Exception } else { final File tmpFile = File.createTempFile("compressionUtilZipCache", ZIP_SUFFIX); try { - FileUtils.FileCopyResult copyResult = FileUtils.retryCopy( + FileUtils.retryCopy( byteSource, tmpFile, shouldRetry, diff --git a/src/main/java/com/metamx/common/Granularity.java b/src/main/java/com/metamx/common/Granularity.java index ee3be518..1e8bf4b1 100644 --- a/src/main/java/com/metamx/common/Granularity.java +++ b/src/main/java/com/metamx/common/Granularity.java @@ -450,9 +450,6 @@ public DateTime toDate(String filePath, Formatter formatter) }, WEEK { - DateTimeFormatter defaultFormat = DateTimeFormat.forPattern("'y'=yyyy/'m'=MM/'d'=dd"); - DateTimeFormatter hiveFormat = DateTimeFormat.forPattern("'dt'=yyyy-MM-dd"); - DateTimeFormatter lowerDefaultFormat = DateTimeFormat.forPattern("'y'=yyyy/'m'=MM/'d'=dd"); @Override public DateTimeFormatter getFormatter(Formatter type) diff --git a/src/main/java/com/metamx/common/RetryUtils.java b/src/main/java/com/metamx/common/RetryUtils.java index 79966b22..873b42ac 100644 --- a/src/main/java/com/metamx/common/RetryUtils.java +++ b/src/main/java/com/metamx/common/RetryUtils.java @@ -23,6 +23,7 @@ import java.util.Random; import java.util.concurrent.Callable; +import java.util.concurrent.ThreadLocalRandom; public class RetryUtils { @@ -82,7 +83,7 @@ private static void awaitNextRetry(final Throwable e, final int nTry, final bool { final long baseSleepMillis = 1000; final long maxSleepMillis = 60000; - final double fuzzyMultiplier = Math.min(Math.max(1 + 0.2 * new Random().nextGaussian(), 0), 2); + final double fuzzyMultiplier = Math.min(Math.max(1 + 0.2 * ThreadLocalRandom.current().nextGaussian(), 0), 2); final long sleepMillis = (long) (Math.min(maxSleepMillis, baseSleepMillis * Math.pow(2, nTry - 1)) * fuzzyMultiplier); if (quiet) { diff --git a/src/main/java/com/metamx/common/StreamUtils.java b/src/main/java/com/metamx/common/StreamUtils.java index cc588e8d..8973b456 100644 --- a/src/main/java/com/metamx/common/StreamUtils.java +++ b/src/main/java/com/metamx/common/StreamUtils.java @@ -132,9 +132,9 @@ public static long copyAndClose(InputStream is, OutputStream os) throws IOExcept public static long copyWithTimeout(InputStream is, OutputStream os, long timeout) throws IOException, TimeoutException { byte[] buffer = new byte[DEFAULT_BUFFER_SIZE]; - int n = 0; + int n; long startTime = System.currentTimeMillis(); - long size = 0l; + long size = 0; while (-1 != (n = is.read(buffer))) { if (System.currentTimeMillis() - startTime > timeout) { throw new TimeoutException(String.format("Copy time has exceeded %,d millis", timeout)); diff --git a/src/main/java/com/metamx/common/StringUtils.java b/src/main/java/com/metamx/common/StringUtils.java index cbe4c0a4..7297d150 100644 --- a/src/main/java/com/metamx/common/StringUtils.java +++ b/src/main/java/com/metamx/common/StringUtils.java @@ -29,7 +29,6 @@ */ public class StringUtils { - private static final Logger log = new Logger(StringUtils.class); @Deprecated // Charset parameters to String are currently slower than the charset's string name public static final Charset UTF8_CHARSET = Charsets.UTF_8; public static final String UTF8_STRING = com.google.common.base.Charsets.UTF_8.toString(); diff --git a/src/main/java/com/metamx/common/guava/Comparators.java b/src/main/java/com/metamx/common/guava/Comparators.java index 1de3f86f..a818cc22 100644 --- a/src/main/java/com/metamx/common/guava/Comparators.java +++ b/src/main/java/com/metamx/common/guava/Comparators.java @@ -40,7 +40,7 @@ public static Comparator inverse(final Comparator baseComp) @Override public int compare(T t, T t1) { - return - baseComp.compare(t, t1); + return baseComp.compare(t1, t); } }; } diff --git a/src/main/java/com/metamx/common/guava/LimitedYieldingAccumulator.java b/src/main/java/com/metamx/common/guava/LimitedYieldingAccumulator.java index 5aa39e2d..075ba99c 100644 --- a/src/main/java/com/metamx/common/guava/LimitedYieldingAccumulator.java +++ b/src/main/java/com/metamx/common/guava/LimitedYieldingAccumulator.java @@ -18,6 +18,7 @@ /** */ +@Deprecated public class LimitedYieldingAccumulator extends YieldingAccumulator { private final int limit; diff --git a/src/main/java/com/metamx/common/parsers/CSVParser.java b/src/main/java/com/metamx/common/parsers/CSVParser.java index 9c980eac..11558ecd 100644 --- a/src/main/java/com/metamx/common/parsers/CSVParser.java +++ b/src/main/java/com/metamx/common/parsers/CSVParser.java @@ -31,7 +31,6 @@ public class CSVParser implements Parser { - private static final Logger log = new Logger(CSVParser.class); private final String listDelimiter; private final Splitter listSplitter; diff --git a/src/main/java/com/metamx/common/parsers/DelimitedParser.java b/src/main/java/com/metamx/common/parsers/DelimitedParser.java index 6729bd20..a9783415 100644 --- a/src/main/java/com/metamx/common/parsers/DelimitedParser.java +++ b/src/main/java/com/metamx/common/parsers/DelimitedParser.java @@ -33,7 +33,6 @@ public class DelimitedParser implements Parser { - private static final Logger log = new Logger(DelimitedParser.class); private static final String DEFAULT_DELIMITER = "\t"; private final String delimiter; diff --git a/src/main/java/com/metamx/common/parsers/JSONPathParser.java b/src/main/java/com/metamx/common/parsers/JSONPathParser.java index 87dbef02..ff9c891f 100644 --- a/src/main/java/com/metamx/common/parsers/JSONPathParser.java +++ b/src/main/java/com/metamx/common/parsers/JSONPathParser.java @@ -44,7 +44,6 @@ public class JSONPathParser implements Parser { private final Map> fieldPathMap; - private final List fieldSpecs; private final boolean useFieldDiscovery; private final ObjectMapper mapper; private final CharsetEncoder enc = Charsets.UTF_8.newEncoder(); @@ -60,7 +59,6 @@ public class JSONPathParser implements Parser */ public JSONPathParser(List fieldSpecs, boolean useFieldDiscovery, ObjectMapper mapper) { - this.fieldSpecs = fieldSpecs; this.fieldPathMap = generateFieldPaths(fieldSpecs); this.useFieldDiscovery = useFieldDiscovery; this.mapper = mapper == null ? new ObjectMapper() : mapper; @@ -145,9 +143,10 @@ private Map> generateFieldPaths(List map, Map document) { - for (String field : document.keySet()) { + for (Map.Entry e : document.entrySet()) { + String field = e.getKey(); if (!map.containsKey(field)) { - Object val = document.get(field); + Object val = e.getValue(); if (val == null) { continue; } diff --git a/src/main/java/com/metamx/common/parsers/TimestampParser.java b/src/main/java/com/metamx/common/parsers/TimestampParser.java index 8561c8da..bc32ce40 100644 --- a/src/main/java/com/metamx/common/parsers/TimestampParser.java +++ b/src/main/java/com/metamx/common/parsers/TimestampParser.java @@ -26,7 +26,6 @@ public class TimestampParser { - private static final Logger log = new Logger(TimestampParser.class); public static Function createTimestampParser( final String format From ab658734040041a59e5e46495c10bfeb4bd670df Mon Sep 17 00:00:00 2001 From: leventov Date: Mon, 10 Oct 2016 18:39:20 +0300 Subject: [PATCH 2/4] Add test for Comparators.inverse() overflow --- .../com/metamx/common/guava/ComparatorsTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/test/java/com/metamx/common/guava/ComparatorsTest.java b/src/test/java/com/metamx/common/guava/ComparatorsTest.java index 8afd92e3..2db32769 100644 --- a/src/test/java/com/metamx/common/guava/ComparatorsTest.java +++ b/src/test/java/com/metamx/common/guava/ComparatorsTest.java @@ -41,6 +41,20 @@ public void testInverse() throws Exception Assert.assertEquals(0, inverted.compare(1, 1)); } + @Test + public void testInverseOverflow() + { + Comparator invertedSimpleIntegerComparator = Comparators.inverse(new Comparator() + { + @Override + public int compare(Integer o1, Integer o2) + { + return o1 - o2; + } + }); + Assert.assertTrue(invertedSimpleIntegerComparator.compare(0, Integer.MIN_VALUE) < 0); + } + @Test public void testIntervalsByStartThenEnd() throws Exception { From b76992d2c8a13c92a1e14ba01e74df4f89b73b15 Mon Sep 17 00:00:00 2001 From: leventov Date: Mon, 10 Oct 2016 18:40:02 +0300 Subject: [PATCH 3/4] Add deprecation, why LimitedYieldingAccumulator is deprecated --- .../com/metamx/common/guava/LimitedYieldingAccumulator.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/com/metamx/common/guava/LimitedYieldingAccumulator.java b/src/main/java/com/metamx/common/guava/LimitedYieldingAccumulator.java index 075ba99c..db863673 100644 --- a/src/main/java/com/metamx/common/guava/LimitedYieldingAccumulator.java +++ b/src/main/java/com/metamx/common/guava/LimitedYieldingAccumulator.java @@ -17,6 +17,8 @@ package com.metamx.common.guava; /** + * @deprecated this class uses expensive volatile counter inside, but it is not thread-safe. It is going to be removed + * in the future. */ @Deprecated public class LimitedYieldingAccumulator extends YieldingAccumulator From 8c7d2b8d6957408c86fe06663e658508d3bbd37b Mon Sep 17 00:00:00 2001 From: leventov Date: Mon, 10 Oct 2016 18:42:34 +0300 Subject: [PATCH 4/4] Remove unused import --- src/main/java/com/metamx/common/RetryUtils.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/metamx/common/RetryUtils.java b/src/main/java/com/metamx/common/RetryUtils.java index 873b42ac..9ace667d 100644 --- a/src/main/java/com/metamx/common/RetryUtils.java +++ b/src/main/java/com/metamx/common/RetryUtils.java @@ -21,7 +21,6 @@ import com.google.common.base.Throwables; import com.metamx.common.logger.Logger; -import java.util.Random; import java.util.concurrent.Callable; import java.util.concurrent.ThreadLocalRandom;