From bc77b04bc7ec9aa68d2d98ce5501b751d6d17ed8 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Tue, 27 Jun 2017 20:24:20 +0100 Subject: [PATCH 01/13] Remove some unnecessary use of boxed types. --- .../benchmark/datagen/BenchmarkColumnValueGenerator.java | 2 +- .../io/druid/benchmark/datagen/RealRoundingDistribution.java | 4 ++-- .../src/main/java/io/druid/query/scan/ScanQueryEngine.java | 2 +- .../io/druid/query/aggregation/TimestampBufferAggregator.java | 2 +- .../io/druid/segment/incremental/OffheapIncrementalIndex.java | 2 +- .../io/druid/segment/incremental/OnheapIncrementalIndex.java | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/datagen/BenchmarkColumnValueGenerator.java b/benchmarks/src/main/java/io/druid/benchmark/datagen/BenchmarkColumnValueGenerator.java index b41672a1e748..3e5eec5f1f67 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/datagen/BenchmarkColumnValueGenerator.java +++ b/benchmarks/src/main/java/io/druid/benchmark/datagen/BenchmarkColumnValueGenerator.java @@ -59,7 +59,7 @@ public Object generateRowValue() int rowSize = schema.getRowSize(); if (nullProbability != null) { - Double randDouble = simpleRng.nextDouble(); + double randDouble = simpleRng.nextDouble(); if (randDouble <= nullProbability) { return null; } diff --git a/benchmarks/src/main/java/io/druid/benchmark/datagen/RealRoundingDistribution.java b/benchmarks/src/main/java/io/druid/benchmark/datagen/RealRoundingDistribution.java index 04c78a486c65..8d767c43e778 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/datagen/RealRoundingDistribution.java +++ b/benchmarks/src/main/java/io/druid/benchmark/datagen/RealRoundingDistribution.java @@ -86,7 +86,7 @@ public void reseedRandomGenerator(long seed) public int sample() { double randomVal = realDist.sample(); - Long longVal = Math.round(randomVal); - return longVal.intValue(); + long longVal = Math.round(randomVal); + return (int) longVal; } } diff --git a/extensions-contrib/scan-query/src/main/java/io/druid/query/scan/ScanQueryEngine.java b/extensions-contrib/scan-query/src/main/java/io/druid/query/scan/ScanQueryEngine.java index 3011f7b2b4bd..53d668b3e111 100644 --- a/extensions-contrib/scan-query/src/main/java/io/druid/query/scan/ScanQueryEngine.java +++ b/extensions-contrib/scan-query/src/main/java/io/druid/query/scan/ScanQueryEngine.java @@ -67,7 +67,7 @@ public Sequence process( } } final boolean hasTimeout = QueryContexts.hasTimeout(query); - final Long timeoutAt = (long) responseContext.get(ScanQueryRunnerFactory.CTX_TIMEOUT_AT); + final long timeoutAt = (long) responseContext.get(ScanQueryRunnerFactory.CTX_TIMEOUT_AT); final long start = System.currentTimeMillis(); final StorageAdapter adapter = segment.asStorageAdapter(); diff --git a/extensions-contrib/time-min-max/src/main/java/io/druid/query/aggregation/TimestampBufferAggregator.java b/extensions-contrib/time-min-max/src/main/java/io/druid/query/aggregation/TimestampBufferAggregator.java index 6ee0bb815aaf..335359e79670 100644 --- a/extensions-contrib/time-min-max/src/main/java/io/druid/query/aggregation/TimestampBufferAggregator.java +++ b/extensions-contrib/time-min-max/src/main/java/io/druid/query/aggregation/TimestampBufferAggregator.java @@ -56,7 +56,7 @@ public void aggregate(ByteBuffer buf, int position) { Long newTime = TimestampAggregatorFactory.convertLong(timestampSpec, selector.get()); if (newTime != null) { - Long prev = buf.getLong(position); + long prev = buf.getLong(position); buf.putLong(position, comparator.compare(prev, newTime) > 0 ? prev: newTime); } } diff --git a/processing/src/main/java/io/druid/segment/incremental/OffheapIncrementalIndex.java b/processing/src/main/java/io/druid/segment/incremental/OffheapIncrementalIndex.java index 598d454a27c3..4040e13b8c92 100644 --- a/processing/src/main/java/io/druid/segment/incremental/OffheapIncrementalIndex.java +++ b/processing/src/main/java/io/druid/segment/incremental/OffheapIncrementalIndex.java @@ -200,7 +200,7 @@ protected Integer addToFacts( throw new IndexSizeExceededException("Maximum number of rows [%d] reached", maxRowCount); } - final Integer rowIndex = indexIncrement.getAndIncrement(); + final int rowIndex = indexIncrement.getAndIncrement(); // note that indexAndOffsets must be updated before facts, because as soon as we update facts // concurrent readers get hold of it and might ask for newly added row diff --git a/processing/src/main/java/io/druid/segment/incremental/OnheapIncrementalIndex.java b/processing/src/main/java/io/druid/segment/incremental/OnheapIncrementalIndex.java index 059d954772cc..0657723ec6b2 100644 --- a/processing/src/main/java/io/druid/segment/incremental/OnheapIncrementalIndex.java +++ b/processing/src/main/java/io/druid/segment/incremental/OnheapIncrementalIndex.java @@ -125,7 +125,7 @@ protected Integer addToFacts( factorizeAggs(metrics, aggs, rowContainer, row); doAggregate(metrics, aggs, rowContainer, row, reportParseExceptions); - final Integer rowIndex = indexIncrement.getAndIncrement(); + final int rowIndex = indexIncrement.getAndIncrement(); concurrentSet(rowIndex, aggs); // Last ditch sanity checks From 7e9a3223a2416ec8e8cd08abeacdc0c687f7e1fe Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Tue, 27 Jun 2017 20:55:45 +0100 Subject: [PATCH 02/13] Fix some incorrect format strings. --- .../java/io/druid/storage/google/GoogleDataSegmentPuller.java | 2 +- .../io/druid/indexing/worker/executor/ExecutorLifecycle.java | 4 ++-- .../query/groupby/epinephelinae/AbstractBufferGrouper.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions-contrib/google-extensions/src/main/java/io/druid/storage/google/GoogleDataSegmentPuller.java b/extensions-contrib/google-extensions/src/main/java/io/druid/storage/google/GoogleDataSegmentPuller.java index e814159b241a..8d1dc66fd9fe 100644 --- a/extensions-contrib/google-extensions/src/main/java/io/druid/storage/google/GoogleDataSegmentPuller.java +++ b/extensions-contrib/google-extensions/src/main/java/io/druid/storage/google/GoogleDataSegmentPuller.java @@ -62,7 +62,7 @@ public void getSegmentFiles(final DataSegment segment, final File outDir) throws public FileUtils.FileCopyResult getSegmentFiles(final String bucket, final String path, File outDir) throws SegmentLoadingException { - LOG.info("Pulling index at path[%s] to outDir[%s]", bucket, path, outDir.getAbsolutePath()); + LOG.info("Pulling index at bucket[%s] path[%s] to outDir[%s]", bucket, path, outDir.getAbsolutePath()); try { prepareOutDir(outDir); diff --git a/indexing-service/src/main/java/io/druid/indexing/worker/executor/ExecutorLifecycle.java b/indexing-service/src/main/java/io/druid/indexing/worker/executor/ExecutorLifecycle.java index 90d0890d0ddf..3036f1f18509 100644 --- a/indexing-service/src/main/java/io/druid/indexing/worker/executor/ExecutorLifecycle.java +++ b/indexing-service/src/main/java/io/druid/indexing/worker/executor/ExecutorLifecycle.java @@ -167,11 +167,11 @@ public void run() // Won't hurt in remote mode, and is required for setting up locks in local mode: try { if (!task.isReady(taskActionClientFactory.create(task))) { - throw new ISE("Task is not ready to run yet!", task.getId()); + throw new ISE("Task[%s] is not ready to run yet!", task.getId()); } } catch (Exception e) { - throw new ISE(e, "Failed to run isReady", task.getId()); + throw new ISE(e, "Failed to run task[%s] isReady", task.getId()); } statusFuture = Futures.transform( diff --git a/processing/src/main/java/io/druid/query/groupby/epinephelinae/AbstractBufferGrouper.java b/processing/src/main/java/io/druid/query/groupby/epinephelinae/AbstractBufferGrouper.java index a0b5e8d10cef..61e9c60d3f14 100644 --- a/processing/src/main/java/io/druid/query/groupby/epinephelinae/AbstractBufferGrouper.java +++ b/processing/src/main/java/io/druid/query/groupby/epinephelinae/AbstractBufferGrouper.java @@ -195,7 +195,7 @@ public void close() aggregator.close(); } catch (Exception e) { - log.warn(e, "Could not close aggregator, skipping.", aggregator); + log.warn(e, "Could not close aggregator, skipping."); } } } From 5f33a2ee1089f1daf4d39ae9450b450b96f054de Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Tue, 4 Jul 2017 09:17:40 +0100 Subject: [PATCH 03/13] Enable IDEA's MalformedFormatString inspection. --- .idea/inspectionProfiles/Druid.xml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.idea/inspectionProfiles/Druid.xml b/.idea/inspectionProfiles/Druid.xml index 5eb60b4ba691..5d889a5af5dd 100644 --- a/.idea/inspectionProfiles/Druid.xml +++ b/.idea/inspectionProfiles/Druid.xml @@ -43,6 +43,10 @@ + + @@ -104,4 +108,4 @@ - \ No newline at end of file + From 05596e5f241602d5ef34959c70d081c0d577d316 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Tue, 4 Jul 2017 09:17:53 +0100 Subject: [PATCH 04/13] Add a Checkstyle check for finding uses of incorrect logging packages. --- codestyle/checkstyle.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/codestyle/checkstyle.xml b/codestyle/checkstyle.xml index 17570da445dd..a18ab37158b6 100644 --- a/codestyle/checkstyle.xml +++ b/codestyle/checkstyle.xml @@ -71,5 +71,10 @@ + + + + + From bc49dca0b636174c3bfd1dc563f2f72687fd135c Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Wed, 5 Jul 2017 13:03:10 +0100 Subject: [PATCH 05/13] Fix some incorrect usages of the metamx logger. --- .../druid/examples/twitter/TwitterSpritzerFirehoseFactory.java | 2 +- .../io/druid/emitter/ambari/metrics/AmbariMetricsEmitter.java | 2 +- .../metrics/WhiteListBasedDruidToTimelineEventConverter.java | 2 +- .../io/druid/metadata/storage/sqlserver/SQLServerConnector.java | 2 +- .../io/druid/query/groupby/resource/GroupByQueryResource.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/examples/src/main/java/io/druid/examples/twitter/TwitterSpritzerFirehoseFactory.java b/examples/src/main/java/io/druid/examples/twitter/TwitterSpritzerFirehoseFactory.java index c7be90b8a2d2..5e1aa28ba817 100644 --- a/examples/src/main/java/io/druid/examples/twitter/TwitterSpritzerFirehoseFactory.java +++ b/examples/src/main/java/io/druid/examples/twitter/TwitterSpritzerFirehoseFactory.java @@ -25,12 +25,12 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; -import com.metamx.common.logger.Logger; import io.druid.data.input.Firehose; import io.druid.data.input.FirehoseFactory; import io.druid.data.input.InputRow; import io.druid.data.input.MapBasedInputRow; import io.druid.data.input.impl.InputRowParser; +import io.druid.java.util.common.logger.Logger; import twitter4j.ConnectionLifeCycleListener; import twitter4j.GeoLocation; import twitter4j.HashtagEntity; diff --git a/extensions-contrib/ambari-metrics-emitter/src/main/java/io/druid/emitter/ambari/metrics/AmbariMetricsEmitter.java b/extensions-contrib/ambari-metrics-emitter/src/main/java/io/druid/emitter/ambari/metrics/AmbariMetricsEmitter.java index 17b08ced868b..585f66757aca 100644 --- a/extensions-contrib/ambari-metrics-emitter/src/main/java/io/druid/emitter/ambari/metrics/AmbariMetricsEmitter.java +++ b/extensions-contrib/ambari-metrics-emitter/src/main/java/io/druid/emitter/ambari/metrics/AmbariMetricsEmitter.java @@ -21,11 +21,11 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.metamx.common.ISE; -import com.metamx.common.logger.Logger; import com.metamx.emitter.core.Emitter; import com.metamx.emitter.core.Event; import com.metamx.emitter.service.AlertEvent; import com.metamx.emitter.service.ServiceMetricEvent; +import io.druid.java.util.common.logger.Logger; import org.apache.hadoop.metrics2.sink.timeline.AbstractTimelineMetricsSink; import org.apache.hadoop.metrics2.sink.timeline.TimelineMetric; import org.apache.hadoop.metrics2.sink.timeline.TimelineMetrics; diff --git a/extensions-contrib/ambari-metrics-emitter/src/main/java/io/druid/emitter/ambari/metrics/WhiteListBasedDruidToTimelineEventConverter.java b/extensions-contrib/ambari-metrics-emitter/src/main/java/io/druid/emitter/ambari/metrics/WhiteListBasedDruidToTimelineEventConverter.java index 8ec31ad9f0ae..0db81b07f068 100644 --- a/extensions-contrib/ambari-metrics-emitter/src/main/java/io/druid/emitter/ambari/metrics/WhiteListBasedDruidToTimelineEventConverter.java +++ b/extensions-contrib/ambari-metrics-emitter/src/main/java/io/druid/emitter/ambari/metrics/WhiteListBasedDruidToTimelineEventConverter.java @@ -33,8 +33,8 @@ import com.google.common.io.CharStreams; import com.google.common.io.Files; import com.metamx.common.ISE; -import com.metamx.common.logger.Logger; import com.metamx.emitter.service.ServiceMetricEvent; +import io.druid.java.util.common.logger.Logger; import org.apache.hadoop.metrics2.sink.timeline.TimelineMetric; import java.io.File; diff --git a/extensions-contrib/sqlserver-metadata-storage/src/main/java/io/druid/metadata/storage/sqlserver/SQLServerConnector.java b/extensions-contrib/sqlserver-metadata-storage/src/main/java/io/druid/metadata/storage/sqlserver/SQLServerConnector.java index 52e0e814e13a..10d28c28df9f 100644 --- a/extensions-contrib/sqlserver-metadata-storage/src/main/java/io/druid/metadata/storage/sqlserver/SQLServerConnector.java +++ b/extensions-contrib/sqlserver-metadata-storage/src/main/java/io/druid/metadata/storage/sqlserver/SQLServerConnector.java @@ -37,8 +37,8 @@ import com.google.common.base.Supplier; import com.google.inject.Inject; -import com.metamx.common.logger.Logger; +import io.druid.java.util.common.logger.Logger; import io.druid.metadata.MetadataStorageConnectorConfig; import io.druid.metadata.MetadataStorageTablesConfig; import io.druid.metadata.SQLMetadataConnector; diff --git a/processing/src/main/java/io/druid/query/groupby/resource/GroupByQueryResource.java b/processing/src/main/java/io/druid/query/groupby/resource/GroupByQueryResource.java index fa993af9c303..03fd7da0331d 100644 --- a/processing/src/main/java/io/druid/query/groupby/resource/GroupByQueryResource.java +++ b/processing/src/main/java/io/druid/query/groupby/resource/GroupByQueryResource.java @@ -19,8 +19,8 @@ package io.druid.query.groupby.resource; -import com.metamx.common.logger.Logger; import io.druid.collections.ResourceHolder; +import io.druid.java.util.common.logger.Logger; import java.io.Closeable; import java.nio.ByteBuffer; From e832e7e98e23f672d01d7c27d269d153166898a0 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Wed, 5 Jul 2017 13:05:26 +0100 Subject: [PATCH 06/13] Bypass incorrect logger Checkstyle check where using the correct logger is not simple. --- .../test/java/io/druid/indexing/kafka/KafkaIndexTaskTest.java | 2 ++ .../io/druid/indexing/common/task/RealtimeIndexTaskTest.java | 2 ++ .../druid/segment/realtime/appenderator/AppenderatorTester.java | 2 ++ .../java/io/druid/server/coordinator/rules/LoadRuleTest.java | 2 ++ 4 files changed, 8 insertions(+) diff --git a/extensions-core/kafka-indexing-service/src/test/java/io/druid/indexing/kafka/KafkaIndexTaskTest.java b/extensions-core/kafka-indexing-service/src/test/java/io/druid/indexing/kafka/KafkaIndexTaskTest.java index fd3761debff0..f96eab3b64f1 100644 --- a/extensions-core/kafka-indexing-service/src/test/java/io/druid/indexing/kafka/KafkaIndexTaskTest.java +++ b/extensions-core/kafka-indexing-service/src/test/java/io/druid/indexing/kafka/KafkaIndexTaskTest.java @@ -37,7 +37,9 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; +//CHECKSTYLE.OFF: Regexp import com.metamx.common.logger.Logger; +//CHECKSTYLE.ON: Regexp import com.metamx.emitter.EmittingLogger; import com.metamx.emitter.core.LoggingEmitter; import com.metamx.emitter.service.ServiceEmitter; diff --git a/indexing-service/src/test/java/io/druid/indexing/common/task/RealtimeIndexTaskTest.java b/indexing-service/src/test/java/io/druid/indexing/common/task/RealtimeIndexTaskTest.java index 82d09be4c44d..32d45f94fc0f 100644 --- a/indexing-service/src/test/java/io/druid/indexing/common/task/RealtimeIndexTaskTest.java +++ b/indexing-service/src/test/java/io/druid/indexing/common/task/RealtimeIndexTaskTest.java @@ -31,7 +31,9 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; +//CHECKSTYLE.OFF: Regexp import com.metamx.common.logger.Logger; +//CHECKSTYLE.ON: Regexp import com.metamx.emitter.EmittingLogger; import com.metamx.emitter.core.LoggingEmitter; import com.metamx.emitter.service.ServiceEmitter; diff --git a/server/src/test/java/io/druid/segment/realtime/appenderator/AppenderatorTester.java b/server/src/test/java/io/druid/segment/realtime/appenderator/AppenderatorTester.java index 71fd3714d23c..ede7b881188c 100644 --- a/server/src/test/java/io/druid/segment/realtime/appenderator/AppenderatorTester.java +++ b/server/src/test/java/io/druid/segment/realtime/appenderator/AppenderatorTester.java @@ -21,7 +21,9 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; +//CHECKSTYLE.OFF: Regexp import com.metamx.common.logger.Logger; +//CHECKSTYLE.ON: Regexp import com.metamx.emitter.EmittingLogger; import com.metamx.emitter.core.LoggingEmitter; import com.metamx.emitter.service.ServiceEmitter; diff --git a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java index 64d91e7c1248..b5ad63142dc3 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java @@ -28,7 +28,9 @@ import com.google.common.collect.Sets; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; +//CHECKSTYLE.OFF: Regexp import com.metamx.common.logger.Logger; +//CHECKSTYLE.ON: Regexp import com.metamx.emitter.EmittingLogger; import com.metamx.emitter.core.LoggingEmitter; import com.metamx.emitter.service.ServiceEmitter; From cc9c0d2330ce9805a60ed4fa750f9c4e0de89e32 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Wed, 5 Jul 2017 15:31:50 +0100 Subject: [PATCH 07/13] Fix some more places where the wrong number of arguments are provided to format strings. --- .../druid/server/coordination/BatchDataSegmentAnnouncer.java | 4 ++-- .../druid/server/metrics/EventReceiverFirehoseRegister.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordination/BatchDataSegmentAnnouncer.java b/server/src/main/java/io/druid/server/coordination/BatchDataSegmentAnnouncer.java index b73322f726e7..20249e015aec 100644 --- a/server/src/main/java/io/druid/server/coordination/BatchDataSegmentAnnouncer.java +++ b/server/src/main/java/io/druid/server/coordination/BatchDataSegmentAnnouncer.java @@ -114,7 +114,7 @@ public DataSegment apply(DataSegment input) public void announceSegment(DataSegment segment) throws IOException { if (segmentLookup.containsKey(segment)) { - log.info("Skipping announcement of segment [%s]. Announcement exists already."); + log.info("Skipping announcement of segment [%s]. Announcement exists already.", segment.getIdentifier()); return; } @@ -222,7 +222,7 @@ public void announceSegments(Iterable segments) throws IOException for (DataSegment ds : segments) { if (segmentLookup.containsKey(ds)) { - log.info("Skipping announcement of segment [%s]. Announcement exists already."); + log.info("Skipping announcement of segment [%s]. Announcement exists already.", ds.getIdentifier()); return; } diff --git a/server/src/main/java/io/druid/server/metrics/EventReceiverFirehoseRegister.java b/server/src/main/java/io/druid/server/metrics/EventReceiverFirehoseRegister.java index 10d6e06bcaba..70542c9f086d 100644 --- a/server/src/main/java/io/druid/server/metrics/EventReceiverFirehoseRegister.java +++ b/server/src/main/java/io/druid/server/metrics/EventReceiverFirehoseRegister.java @@ -50,7 +50,7 @@ public void unregister(String serviceName) { log.info("Unregistering EventReceiverFirehoseMetric for service [%s]", serviceName); if (metrics.remove(serviceName) == null) { - log.warn("Unregistering a non-exist service. Service [%s] never exists."); + log.warn("Unregistering a non-exist service. Service [%s] never exists.", serviceName); } } } From 54d29e48febe16b7a332969f5b9b40eb2bcfdf2f Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Wed, 5 Jul 2017 19:46:31 +0100 Subject: [PATCH 08/13] Suppress `MalformedFormatString` inspection on legacy logging test. --- .../test/java/io/druid/java/util/common/logger/LoggerTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java b/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java index 1004c916aa92..4df693537bbe 100644 --- a/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java +++ b/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java @@ -31,6 +31,7 @@ public void testLogWithCrazyMessages() log.warn(message); } + /** @noinspection MalformedFormatString */ @Test public void testLegacyLogging() { From 58ad43669ffe26f4ca9c226834755604f35d59c4 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Thu, 6 Jul 2017 01:33:41 +0100 Subject: [PATCH 09/13] Use @SuppressWarnings rather than a noinspection suppression comment. --- .../test/java/io/druid/java/util/common/logger/LoggerTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java b/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java index 4df693537bbe..e45cdecaa3f0 100644 --- a/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java +++ b/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java @@ -31,8 +31,7 @@ public void testLogWithCrazyMessages() log.warn(message); } - /** @noinspection MalformedFormatString */ - @Test + @SuppressWarnings("MalformedFormatString") public void testLegacyLogging() { final Logger log = new Logger(LoggerTest.class); From 26746d19acb59a9e49ce5465fa7a7bd76e1de73e Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Thu, 6 Jul 2017 10:21:12 +0100 Subject: [PATCH 10/13] Fix some more incorrect format strings. --- .../main/java/io/druid/collections/StupidPool.java | 2 +- .../firehose/rocketmq/RocketMQFirehoseFactory.java | 12 ++++++------ .../io/druid/firehose/kafka/KafkaSimpleConsumer.java | 2 +- .../io/druid/tests/hadoop/ITHadoopIndexTest.java | 2 +- .../main/java/io/druid/query/QueryRunnerHelper.java | 2 +- .../java/io/druid/query/topn/TopNQueryEngine.java | 2 +- .../io/druid/client/cache/ByteCountingLRUMap.java | 2 +- .../CoordinatorBasedSegmentHandoffNotifier.java | 2 +- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/common/src/main/java/io/druid/collections/StupidPool.java b/common/src/main/java/io/druid/collections/StupidPool.java index dd1918732de1..a815325ca37b 100644 --- a/common/src/main/java/io/druid/collections/StupidPool.java +++ b/common/src/main/java/io/druid/collections/StupidPool.java @@ -163,7 +163,7 @@ private void impossibleOffsetFailed(T object, ObjectId objectId, Cleaner cleaner cleaner.clean(); log.error( new ISE("Queue offer failed"), - "Could not offer object [%s] back into the queue in [%s], objectId [%s]", + "Could not offer object [%s] back into the queue, objectId [%s]", object, objectId ); diff --git a/extensions-contrib/druid-rocketmq/src/main/java/io/druid/firehose/rocketmq/RocketMQFirehoseFactory.java b/extensions-contrib/druid-rocketmq/src/main/java/io/druid/firehose/rocketmq/RocketMQFirehoseFactory.java index 8ee403a9484f..8075ae46defd 100644 --- a/extensions-contrib/druid-rocketmq/src/main/java/io/druid/firehose/rocketmq/RocketMQFirehoseFactory.java +++ b/extensions-contrib/druid-rocketmq/src/main/java/io/druid/firehose/rocketmq/RocketMQFirehoseFactory.java @@ -191,7 +191,7 @@ public Firehose connect( pullMessageService.start(); } catch (MQClientException e) { - LOGGER.error("Failed to start DefaultMQPullConsumer", e); + LOGGER.error(e, "Failed to start DefaultMQPullConsumer"); throw new IOException("Failed to start RocketMQ client", e); } @@ -228,7 +228,7 @@ public boolean hasMore() } } catch (MQClientException e) { - LOGGER.error("Failed to fetch consume offset for queue: {}", entry.getKey()); + LOGGER.error("Failed to fetch consume offset for queue: %s", entry.getKey()); } } } @@ -241,7 +241,7 @@ public boolean hasMore() hasMore = true; } catch (InterruptedException e) { - LOGGER.error("CountDownLatch await got interrupted", e); + LOGGER.error(e, "CountDownLatch await got interrupted"); } } return hasMore; @@ -448,7 +448,7 @@ private void doPull() case OFFSET_ILLEGAL: LOGGER.error( - "Bad Pull Request: Offset is illegal. Offset used: {}", + "Bad Pull Request: Offset is illegal. Offset used: %d", pullRequest.getNextBeginOffset() ); break; @@ -458,7 +458,7 @@ private void doPull() } } catch (MQClientException | RemotingException | MQBrokerException | InterruptedException e) { - LOGGER.error("Failed to pull message from broker.", e); + LOGGER.error(e, "Failed to pull message from broker."); } finally { pullRequest.getCountDownLatch().countDown(); @@ -485,7 +485,7 @@ public void run() Thread.sleep(10); } catch (InterruptedException e) { - LOGGER.error("", e); + LOGGER.error(e, ""); } synchronized (this) { diff --git a/extensions-contrib/kafka-eight-simpleConsumer/src/main/java/io/druid/firehose/kafka/KafkaSimpleConsumer.java b/extensions-contrib/kafka-eight-simpleConsumer/src/main/java/io/druid/firehose/kafka/KafkaSimpleConsumer.java index 693e1defb7d5..7a81e47d0375 100644 --- a/extensions-contrib/kafka-eight-simpleConsumer/src/main/java/io/druid/firehose/kafka/KafkaSimpleConsumer.java +++ b/extensions-contrib/kafka-eight-simpleConsumer/src/main/java/io/druid/firehose/kafka/KafkaSimpleConsumer.java @@ -240,7 +240,7 @@ public Iterable fetch(long offset, int timeoutMs) throws } catch (Exception e) { ensureNotInterrupted(e); - log.warn(e, "caught exception in fetch {} - {}", topic, partitionId); + log.warn(e, "caught exception in fetch %s - %d", topic, partitionId); response = null; } diff --git a/integration-tests/src/test/java/io/druid/tests/hadoop/ITHadoopIndexTest.java b/integration-tests/src/test/java/io/druid/tests/hadoop/ITHadoopIndexTest.java index fe4450091922..9b50d027661d 100644 --- a/integration-tests/src/test/java/io/druid/tests/hadoop/ITHadoopIndexTest.java +++ b/integration-tests/src/test/java/io/druid/tests/hadoop/ITHadoopIndexTest.java @@ -106,7 +106,7 @@ public void afterClass() unloadAndKillData(BATCH_DATASOURCE); } catch (Exception e) { - LOG.warn(e, "exception while removing segments: [%s]"); + LOG.warn(e, "exception while removing segments"); } } } diff --git a/processing/src/main/java/io/druid/query/QueryRunnerHelper.java b/processing/src/main/java/io/druid/query/QueryRunnerHelper.java index f224cb4e98ad..04224e4d797c 100644 --- a/processing/src/main/java/io/druid/query/QueryRunnerHelper.java +++ b/processing/src/main/java/io/druid/query/QueryRunnerHelper.java @@ -64,7 +64,7 @@ public static Sequence> makeCursorBasedQuery( @Override public Result apply(Cursor input) { - log.debug("Running over cursor[%s]", adapter.getInterval(), input.getTime()); + log.debug("Running over cursor[%s]", input.getTime()); return mapFn.apply(input); } } diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java b/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java index c1bb74717585..84ae8e9d9669 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java @@ -94,7 +94,7 @@ public Sequence> query( @Override public Result apply(Cursor input) { - log.debug("Running over cursor[%s]", adapter.getInterval(), input.getTime()); + log.debug("Running over cursor[%s]", input.getTime()); if (queryMetrics != null) { queryMetrics.cursor(input); } diff --git a/server/src/main/java/io/druid/client/cache/ByteCountingLRUMap.java b/server/src/main/java/io/druid/client/cache/ByteCountingLRUMap.java index 09cb0eedab24..b9a287e11fa8 100644 --- a/server/src/main/java/io/druid/client/cache/ByteCountingLRUMap.java +++ b/server/src/main/java/io/druid/client/cache/ByteCountingLRUMap.java @@ -88,7 +88,7 @@ public byte[] put(ByteBuffer key, byte[] value) if (logEvictions && evictionCount.get() % logEvictionCount == 0) { log.info( "Evicting %,dth element. Size[%,d], numBytes[%,d], averageSize[%,d]", - evictionCount, + evictionCount.get(), size(), numBytes.get(), numBytes.get() / size() diff --git a/server/src/main/java/io/druid/segment/realtime/plumber/CoordinatorBasedSegmentHandoffNotifier.java b/server/src/main/java/io/druid/segment/realtime/plumber/CoordinatorBasedSegmentHandoffNotifier.java index b2bdead3ef2f..0489e1745d72 100644 --- a/server/src/main/java/io/druid/segment/realtime/plumber/CoordinatorBasedSegmentHandoffNotifier.java +++ b/server/src/main/java/io/druid/segment/realtime/plumber/CoordinatorBasedSegmentHandoffNotifier.java @@ -127,7 +127,7 @@ void checkForSegmentHandoffs() catch (Throwable t) { log.error( t, - "Exception while checking handoff for dataSource[%s] Segment[%s], Will try again after [%d]secs", + "Exception while checking handoff for dataSource[%s], Will try again after [%d]secs", dataSource, pollDurationMillis ); From 89a3245e94013b9d8c82bec3bea5a37c0651b1f4 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Thu, 6 Jul 2017 10:26:47 +0100 Subject: [PATCH 11/13] Suppress some more incorrect format string warnings where the incorrect string is intentional. --- .../test/java/io/druid/java/util/common/StringUtilsTest.java | 1 + .../test/java/io/druid/java/util/common/logger/LoggerTest.java | 2 ++ 2 files changed, 3 insertions(+) diff --git a/java-util/src/test/java/io/druid/java/util/common/StringUtilsTest.java b/java-util/src/test/java/io/druid/java/util/common/StringUtilsTest.java index 3b2e9c353bb6..2ba4875ee9a4 100644 --- a/java-util/src/test/java/io/druid/java/util/common/StringUtilsTest.java +++ b/java-util/src/test/java/io/druid/java/util/common/StringUtilsTest.java @@ -108,6 +108,7 @@ public void testCharsetShowsUpAsDeprecated() Assert.assertNotNull(StringUtils.UTF8_CHARSET); } + @SuppressWarnings("MalformedFormatString") @Test public void testNonStrictFormat() { diff --git a/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java b/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java index e45cdecaa3f0..76af70bcf6ea 100644 --- a/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java +++ b/java-util/src/test/java/io/druid/java/util/common/logger/LoggerTest.java @@ -23,6 +23,7 @@ public class LoggerTest { + @SuppressWarnings("MalformedFormatString") @Test public void testLogWithCrazyMessages() { @@ -32,6 +33,7 @@ public void testLogWithCrazyMessages() } @SuppressWarnings("MalformedFormatString") + @Test public void testLegacyLogging() { final Logger log = new Logger(LoggerTest.class); From 878b3930b49429f57f44603e315e97991ed76d16 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Fri, 7 Jul 2017 17:41:56 +0100 Subject: [PATCH 12/13] Log the aggregator when closing it fails. --- .../query/groupby/epinephelinae/AbstractBufferGrouper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processing/src/main/java/io/druid/query/groupby/epinephelinae/AbstractBufferGrouper.java b/processing/src/main/java/io/druid/query/groupby/epinephelinae/AbstractBufferGrouper.java index 61e9c60d3f14..7d847b2b06e4 100644 --- a/processing/src/main/java/io/druid/query/groupby/epinephelinae/AbstractBufferGrouper.java +++ b/processing/src/main/java/io/druid/query/groupby/epinephelinae/AbstractBufferGrouper.java @@ -195,7 +195,7 @@ public void close() aggregator.close(); } catch (Exception e) { - log.warn(e, "Could not close aggregator, skipping."); + log.warn(e, "Could not close aggregator [%s], skipping.", aggregator); } } } From b151c98b8e95e43cd11713d9daf9db8f5ed2ff62 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Fri, 7 Jul 2017 17:43:01 +0100 Subject: [PATCH 13/13] Remove some unneeded log lines. --- processing/src/main/java/io/druid/query/QueryRunnerHelper.java | 1 - .../src/main/java/io/druid/query/topn/TopNQueryEngine.java | 1 - 2 files changed, 2 deletions(-) diff --git a/processing/src/main/java/io/druid/query/QueryRunnerHelper.java b/processing/src/main/java/io/druid/query/QueryRunnerHelper.java index 04224e4d797c..62b7947c58eb 100644 --- a/processing/src/main/java/io/druid/query/QueryRunnerHelper.java +++ b/processing/src/main/java/io/druid/query/QueryRunnerHelper.java @@ -64,7 +64,6 @@ public static Sequence> makeCursorBasedQuery( @Override public Result apply(Cursor input) { - log.debug("Running over cursor[%s]", input.getTime()); return mapFn.apply(input); } } diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java b/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java index 84ae8e9d9669..5d30c664f641 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java @@ -94,7 +94,6 @@ public Sequence> query( @Override public Result apply(Cursor input) { - log.debug("Running over cursor[%s]", input.getTime()); if (queryMetrics != null) { queryMetrics.cursor(input); }