From e98684e87029172ca79be5f55a5a4b68b794604a Mon Sep 17 00:00:00 2001 From: Roman Leventov Date: Thu, 23 May 2019 14:18:29 +0200 Subject: [PATCH 1/2] Fix some problems reported by PVS-Studio --- .gitignore | 1 + .../druid/benchmark/query/SelectBenchmark.java | 3 +-- .../druid/common/config/Log4jShutdown.java | 11 +++-------- .../druid/storage/azure/AzureTaskLogs.java | 6 +++--- .../firehose/kafka/KafkaSimpleConsumer.java | 4 +--- .../segment/MapVirtualColumnSelectTest.java | 3 +-- ...asicAuthenticatorMetadataStorageUpdater.java | 2 +- ...orBasicAuthorizerMetadataStorageUpdater.java | 2 +- .../storage/google/GoogleDataSegmentKiller.java | 2 +- .../druid/storage/google/GoogleUtils.java | 9 --------- .../google/GoogleDataSegmentKillerTest.java | 3 ++- .../histogram/ApproximateHistogram.java | 2 +- .../data/input/orc/OrcStructConverter.java | 1 - .../avro/ParquetAvroHadoopInputRowParser.java | 2 +- .../parquet/simple/ParquetGroupConverter.java | 2 +- .../input/protobuf/ProtoTestEventWrapper.java | 3 +-- .../druid/storage/s3/S3DataSegmentKiller.java | 2 +- .../org/apache/druid/storage/s3/S3Utils.java | 17 ----------------- .../hll/HyperLogLogCollectorBenchmark.java | 2 +- .../indexing/overlord/RemoteTaskRunner.java | 4 ++-- .../overlord/hrtr/HttpRemoteTaskRunnerTest.java | 2 +- .../clients/CoordinatorResourceTestClient.java | 1 + .../query/select/SelectQueryQueryToolChest.java | 7 ++----- .../aggregation/AggregationTestHelper.java | 6 ++---- .../druid/query/search/SearchBinaryFnTest.java | 2 +- .../select/MultiSegmentSelectQueryTest.java | 3 +-- .../select/SelectQueryQueryToolChestTest.java | 3 +-- .../query/select/SelectQueryRunnerTest.java | 3 +-- .../org/apache/druid/segment/TestHelper.java | 16 ++++++---------- .../druid/client/selector/ServerSelector.java | 2 +- .../server/http/HostAndPortWithScheme.java | 2 +- .../client/CachingClusteredClientTest.java | 6 ++---- .../client/CachingClusteredClientTestUtils.java | 3 +-- .../client/cache/ByteCountingLRUMapTest.java | 8 ++++---- .../CuratorDruidCoordinatorTest.java | 5 +---- .../coordinator/DruidCoordinatorTest.java | 5 +---- .../coordinator/HttpLoadQueuePeonTest.java | 5 +---- .../server/coordinator/LoadQueuePeonTest.java | 10 ++-------- .../server/coordinator/LoadQueuePeonTester.java | 5 +---- .../coordinator/TestDruidCoordinatorConfig.java | 5 +---- .../DruidCoordinatorSegmentKillerTest.java | 5 +---- .../security/SecurityResourceFilterTest.java | 2 +- .../druid/sql/calcite/util/CalciteTests.java | 3 +-- 43 files changed, 58 insertions(+), 132 deletions(-) diff --git a/.gitignore b/.gitignore index 40e0adb4bcb2..b3d45cf6e36f 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ target .classpath .idea .project +.PVS-Studio .settings/ *.log *.DS_Store diff --git a/benchmarks/src/main/java/org/apache/druid/benchmark/query/SelectBenchmark.java b/benchmarks/src/main/java/org/apache/druid/benchmark/query/SelectBenchmark.java index 28bbb1ff4a85..522e006d53d0 100644 --- a/benchmarks/src/main/java/org/apache/druid/benchmark/query/SelectBenchmark.java +++ b/benchmarks/src/main/java/org/apache/druid/benchmark/query/SelectBenchmark.java @@ -233,8 +233,7 @@ public void setup() throws IOException factory = new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( JSON_MAPPER, - QueryBenchmarkUtil.noopIntervalChunkingQueryRunnerDecorator(), - selectConfigSupplier + QueryBenchmarkUtil.noopIntervalChunkingQueryRunnerDecorator() ), new SelectQueryEngine(), QueryBenchmarkUtil.NOOP_QUERYWATCHER diff --git a/core/src/main/java/org/apache/druid/common/config/Log4jShutdown.java b/core/src/main/java/org/apache/druid/common/config/Log4jShutdown.java index cdfad64a5f6c..918970c015a9 100644 --- a/core/src/main/java/org/apache/druid/common/config/Log4jShutdown.java +++ b/core/src/main/java/org/apache/druid/common/config/Log4jShutdown.java @@ -158,7 +158,9 @@ private SynchronizedStateHolder(State initial) private synchronized boolean compareAndSet(State expected, State transition) { if (current == expected) { - return transition(transition); + current = transition; + notifyAll(); + return true; } return false; } @@ -189,13 +191,6 @@ private synchronized State waitForTransition(State expected, State transition, l return current; } - private synchronized boolean transition(State transition) - { - current = transition; - notifyAll(); - return true; - } - private synchronized State get() { return current; diff --git a/extensions-contrib/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureTaskLogs.java b/extensions-contrib/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureTaskLogs.java index d75d20083940..0196febea5bd 100644 --- a/extensions-contrib/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureTaskLogs.java +++ b/extensions-contrib/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureTaskLogs.java @@ -53,7 +53,7 @@ public void pushTaskLog(final String taskid, final File logFile) { final String taskKey = getTaskLogKey(taskid); log.info("Pushing task log %s to: %s", logFile, taskKey); - pushTaskFile(taskid, logFile, taskKey); + pushTaskFile(logFile, taskKey); } @Override @@ -61,10 +61,10 @@ public void pushTaskReports(String taskid, File reportFile) { final String taskKey = getTaskReportsKey(taskid); log.info("Pushing task reports %s to: %s", reportFile, taskKey); - pushTaskFile(taskid, reportFile, taskKey); + pushTaskFile(reportFile, taskKey); } - private void pushTaskFile(final String taskId, final File logFile, String taskKey) + private void pushTaskFile(final File logFile, String taskKey) { try { AzureUtils.retryAzureOperation( diff --git a/extensions-contrib/kafka-eight-simpleConsumer/src/main/java/org/apache/druid/firehose/kafka/KafkaSimpleConsumer.java b/extensions-contrib/kafka-eight-simpleConsumer/src/main/java/org/apache/druid/firehose/kafka/KafkaSimpleConsumer.java index 25fc8de15daf..ce3028ff1131 100644 --- a/extensions-contrib/kafka-eight-simpleConsumer/src/main/java/org/apache/druid/firehose/kafka/KafkaSimpleConsumer.java +++ b/extensions-contrib/kafka-eight-simpleConsumer/src/main/java/org/apache/druid/firehose/kafka/KafkaSimpleConsumer.java @@ -60,8 +60,6 @@ public class KafkaSimpleConsumer { - public static final List EMPTY_MSGS = new ArrayList<>(); - private static final Logger log = new Logger(KafkaSimpleConsumer.class); private final List allBrokers; @@ -274,7 +272,7 @@ public Iterable fetch(long offset, int timeoutMs) throws } } - return response != null ? filterAndDecode(response.messageSet(topic, partitionId), offset) : EMPTY_MSGS; + return filterAndDecode(response.messageSet(topic, partitionId), offset); } private void stopConsumer() diff --git a/extensions-contrib/virtual-columns/src/test/java/org/apache/druid/segment/MapVirtualColumnSelectTest.java b/extensions-contrib/virtual-columns/src/test/java/org/apache/druid/segment/MapVirtualColumnSelectTest.java index f248bec0d3bb..4dd24c4b32e6 100644 --- a/extensions-contrib/virtual-columns/src/test/java/org/apache/druid/segment/MapVirtualColumnSelectTest.java +++ b/extensions-contrib/virtual-columns/src/test/java/org/apache/druid/segment/MapVirtualColumnSelectTest.java @@ -70,8 +70,7 @@ public static Iterable constructorFeeder() throws IOException SelectQueryRunnerFactory factory = new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( new DefaultObjectMapper(), - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - selectConfigSupplier + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ), new SelectQueryEngine(), QueryRunnerTestHelper.NOOP_QUERYWATCHER diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java index 941b4a38a0e4..49a1ab332370 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java @@ -91,7 +91,7 @@ public CoordinatorBasicAuthenticatorMetadataStorageUpdater( BasicAuthCommonCacheConfig commonCacheConfig, @Smile ObjectMapper objectMapper, BasicAuthenticatorCacheNotifier cacheNotifier, - ConfigManager configManager // ConfigManager creates the db table we need, set a dependency here + ConfigManager configManager // -V6022: ConfigManager creates the db table we need, set a dependency here ) { this.exec = Execs.scheduledSingleThreaded("CoordinatorBasicAuthenticatorMetadataStorageUpdater-Exec--%d"); diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/db/updater/CoordinatorBasicAuthorizerMetadataStorageUpdater.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/db/updater/CoordinatorBasicAuthorizerMetadataStorageUpdater.java index 5c80ec7d9ba9..e2b5849aad3e 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/db/updater/CoordinatorBasicAuthorizerMetadataStorageUpdater.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/db/updater/CoordinatorBasicAuthorizerMetadataStorageUpdater.java @@ -107,7 +107,7 @@ public CoordinatorBasicAuthorizerMetadataStorageUpdater( BasicAuthCommonCacheConfig commonCacheConfig, @Smile ObjectMapper objectMapper, BasicAuthorizerCacheNotifier cacheNotifier, - ConfigManager configManager // ConfigManager creates the db table we need, set a dependency here + ConfigManager configManager // -V6022: ConfigManager creates the db table we need, set a dependency here ) { this.exec = Execs.scheduledSingleThreaded("CoordinatorBasicAuthorizerMetadataStorageUpdater-Exec--%d"); diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java index be1690126684..88ad394c370a 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java @@ -52,7 +52,7 @@ public void kill(DataSegment segment) throws SegmentLoadingException Map loadSpec = segment.getLoadSpec(); final String bucket = MapUtils.getString(loadSpec, "bucket"); final String indexPath = MapUtils.getString(loadSpec, "path"); - final String descriptorPath = indexPath.substring(0, indexPath.lastIndexOf('/')) + "/descriptor.json"; + final String descriptorPath = indexPath.substring(0, indexPath.lastIndexOf('/')) + "/descriptor.json"; //-V6009 try { deleteIfPresent(bucket, indexPath); diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleUtils.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleUtils.java index 6f23f9ca9978..c3d3e4acb90a 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleUtils.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleUtils.java @@ -25,15 +25,6 @@ public class GoogleUtils { - public static String toFilename(String path) - { - return path.substring(path.lastIndexOf('/') + 1); // characters after last '/' - } - - public static String indexZipForSegmentPath(String path) - { - return path.substring(0, path.lastIndexOf('/')) + "/index.zip"; - } public static boolean isRetryable(Throwable t) { diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java index 6ab2a1a4bb4a..b65285655449 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java @@ -40,7 +40,8 @@ public class GoogleDataSegmentKillerTest extends EasyMockSupport { private static final String bucket = "bucket"; private static final String indexPath = "test/2015-04-12T00:00:00.000Z_2015-04-13T00:00:00.000Z/1/0/index.zip"; - private static final String descriptorPath = indexPath.substring(0, indexPath.lastIndexOf('/')) + "/descriptor.json"; + private static final String descriptorPath = + indexPath.substring(0, indexPath.lastIndexOf('/')) + "/descriptor.json"; //-V6009 private static final DataSegment dataSegment = new DataSegment( "test", diff --git a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java index 0135c8de18d3..57fd51a53b87 100644 --- a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java +++ b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java @@ -419,7 +419,7 @@ protected void mergeInsert(final int mergeAt, int insertAt, final float v, final // use unused slot to shift array left or right and make space for the new bin to insert if (insertAt < unusedIndex) { shiftRight(insertAt, unusedIndex); - } else if (insertAt >= unusedIndex) { + } else { // insertAt >= unusedIndex shiftLeft(unusedIndex, insertAt - 1); insertAt--; } diff --git a/extensions-core/orc-extensions/src/main/java/org/apache/druid/data/input/orc/OrcStructConverter.java b/extensions-core/orc-extensions/src/main/java/org/apache/druid/data/input/orc/OrcStructConverter.java index 20fbf0697203..d27cc3159270 100644 --- a/extensions-core/orc-extensions/src/main/java/org/apache/druid/data/input/orc/OrcStructConverter.java +++ b/extensions-core/orc-extensions/src/main/java/org/apache/druid/data/input/orc/OrcStructConverter.java @@ -166,7 +166,6 @@ Object convertRootField(OrcStruct struct, String fieldName) fieldIndexCache.put(fields.get(i), i); } } - WritableComparable wc = struct.getFieldValue(fieldName); int fieldIndex = fieldIndexCache.getOrDefault(fieldName, -1); diff --git a/extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/avro/ParquetAvroHadoopInputRowParser.java b/extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/avro/ParquetAvroHadoopInputRowParser.java index a297b94e5a1f..1658d1717582 100755 --- a/extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/avro/ParquetAvroHadoopInputRowParser.java +++ b/extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/avro/ParquetAvroHadoopInputRowParser.java @@ -66,7 +66,7 @@ public ParquetAvroHadoopInputRowParser( this.binaryAsString = binaryAsString == null ? false : binaryAsString; final JSONPathSpec flattenSpec; - if (parseSpec != null && (parseSpec instanceof AvroParseSpec)) { + if (parseSpec instanceof AvroParseSpec) { flattenSpec = ((AvroParseSpec) parseSpec).getFlattenSpec(); } else { flattenSpec = JSONPathSpec.DEFAULT; diff --git a/extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/simple/ParquetGroupConverter.java b/extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/simple/ParquetGroupConverter.java index 0aed764f127b..304384ca8986 100644 --- a/extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/simple/ParquetGroupConverter.java +++ b/extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/simple/ParquetGroupConverter.java @@ -397,7 +397,7 @@ private static Object convertPrimitiveField(Group g, int fieldIndex, int index, return bytes; } default: - throw new RE("Unknown primitive conversion: %s", ot.name()); + throw new RE("Unknown primitive conversion: %s", pt.getPrimitiveTypeName()); } } } diff --git a/extensions-core/protobuf-extensions/src/test/java/org/apache/druid/data/input/protobuf/ProtoTestEventWrapper.java b/extensions-core/protobuf-extensions/src/test/java/org/apache/druid/data/input/protobuf/ProtoTestEventWrapper.java index bd7662c92191..1aac2a2aeb1b 100644 --- a/extensions-core/protobuf-extensions/src/test/java/org/apache/druid/data/input/protobuf/ProtoTestEventWrapper.java +++ b/extensions-core/protobuf-extensions/src/test/java/org/apache/druid/data/input/protobuf/ProtoTestEventWrapper.java @@ -710,8 +710,7 @@ public boolean equals(final java.lang.Object obj) } org.apache.druid.data.input.protobuf.ProtoTestEventWrapper.ProtoTestEvent.Foo other = (org.apache.druid.data.input.protobuf.ProtoTestEventWrapper.ProtoTestEvent.Foo) obj; - boolean result = true; - result = result && (hasBar() == other.hasBar()); + boolean result = hasBar() == other.hasBar(); if (hasBar()) { result = result && getBar() .equals(other.getBar()); diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java index 7ddf540173a3..cb2d6ed4ac98 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java @@ -70,7 +70,7 @@ public void kill(DataSegment segment) throws SegmentLoadingException private static String descriptorPathForSegmentPath(String s3Path) { - return s3Path.substring(0, s3Path.lastIndexOf('/')) + "/descriptor.json"; + return s3Path.substring(0, s3Path.lastIndexOf('/')) + "/descriptor.json"; //-V6009 } @Override diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3Utils.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3Utils.java index 1b6defd23d97..58ecdced6995 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3Utils.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3Utils.java @@ -172,23 +172,6 @@ public static String constructSegmentPath(String baseKey, String storageDir) ) + "/index.zip"; } - static String indexZipForSegmentPath(String s3Path) - { - return s3Path.substring(0, s3Path.lastIndexOf('/')) + "/index.zip"; - } - - static String toFilename(String key) - { - return toFilename(key, ""); - } - - static String toFilename(String key, final String suffix) - { - String filename = key.substring(key.lastIndexOf('/') + 1); // characters after last '/' - filename = filename.substring(0, filename.length() - suffix.length()); // remove the suffix from the end - return filename; - } - static AccessControlList grantFullControlToBucketOwner(ServerSideEncryptingAmazonS3 s3Client, String bucket) { final AccessControlList acl = s3Client.getBucketAcl(bucket); diff --git a/hll/src/test/java/org/apache/druid/hll/HyperLogLogCollectorBenchmark.java b/hll/src/test/java/org/apache/druid/hll/HyperLogLogCollectorBenchmark.java index 66ea18e87a21..6b6ba9609ded 100644 --- a/hll/src/test/java/org/apache/druid/hll/HyperLogLogCollectorBenchmark.java +++ b/hll/src/test/java/org/apache/druid/hll/HyperLogLogCollectorBenchmark.java @@ -154,7 +154,7 @@ public double timeFold(int reps) final ByteBuffer buf = allocateEmptyHLLBuffer(targetIsDirect, alignTarget, 0); for (int k = 0; k < reps; ++k) { - for (int i = 0; i < count; ++i) { + for (int i = 0; i < count; ++i) { //-V6017: The 'k' counter is not used the nested loop because it's just reps. final int pos = positions[i]; final int size = sizes[i]; diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/RemoteTaskRunner.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/RemoteTaskRunner.java index fbe64f2a9533..927f03efbfe1 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/RemoteTaskRunner.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/RemoteTaskRunner.java @@ -724,8 +724,8 @@ private void cleanup(final String taskId) return; } final RemoteTaskRunnerWorkItem removed = completeTasks.remove(taskId); - final Worker worker = removed.getWorker(); - if (removed == null || worker == null) { + final Worker worker; + if (removed == null || (worker = removed.getWorker()) == null) { log.makeAlert("WTF?! Asked to cleanup nonexistent task") .addData("taskId", taskId) .emit(); diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java index b62a536ad1f0..2a97b5906499 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java @@ -1285,7 +1285,7 @@ private static WorkerHolder createWorkerHolder( { return new WorkerHolder(smileMapper, httpClient, config, workersSyncExec, listener, worker) { - private final String workerHost = worker.getHost().substring(0, worker.getHost().indexOf(':')); + private final String workerHost = worker.getHost().substring(0, worker.getHost().indexOf(':')); //-V6009 private final int workerPort = Integer.parseInt(worker.getHost().substring(worker.getHost().indexOf(':') + 1)); @Override diff --git a/integration-tests/src/main/java/org/apache/druid/testing/clients/CoordinatorResourceTestClient.java b/integration-tests/src/main/java/org/apache/druid/testing/clients/CoordinatorResourceTestClient.java index babb9e3ead3d..7b73efb09791 100644 --- a/integration-tests/src/main/java/org/apache/druid/testing/clients/CoordinatorResourceTestClient.java +++ b/integration-tests/src/main/java/org/apache/druid/testing/clients/CoordinatorResourceTestClient.java @@ -241,6 +241,7 @@ public Map initializeLookups(String filePath) throws Exception ); } + @SuppressWarnings("unused") // It's unclear whether this call to readValue() has desirable side effects. Map results = jsonMapper.readValue( response.getContent(), new TypeReference>(){} diff --git a/processing/src/main/java/org/apache/druid/query/select/SelectQueryQueryToolChest.java b/processing/src/main/java/org/apache/druid/query/select/SelectQueryQueryToolChest.java index 45ebb95a52dd..9a5fb5a79a97 100644 --- a/processing/src/main/java/org/apache/druid/query/select/SelectQueryQueryToolChest.java +++ b/processing/src/main/java/org/apache/druid/query/select/SelectQueryQueryToolChest.java @@ -24,7 +24,6 @@ import com.google.common.base.Function; import com.google.common.base.Functions; import com.google.common.base.Preconditions; -import com.google.common.base.Supplier; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Ordering; @@ -82,18 +81,16 @@ public class SelectQueryQueryToolChest extends QueryToolChest configSupplier + IntervalChunkingQueryRunnerDecorator intervalChunkingQueryRunnerDecorator ) { - this(jsonMapper, intervalChunkingQueryRunnerDecorator, configSupplier, DefaultSelectQueryMetricsFactory.instance()); + this(jsonMapper, intervalChunkingQueryRunnerDecorator, DefaultSelectQueryMetricsFactory.instance()); } @Inject public SelectQueryQueryToolChest( ObjectMapper jsonMapper, IntervalChunkingQueryRunnerDecorator intervalChunkingQueryRunnerDecorator, - Supplier configSupplier, SelectQueryMetricsFactory queryMetricsFactory ) { diff --git a/processing/src/test/java/org/apache/druid/query/aggregation/AggregationTestHelper.java b/processing/src/test/java/org/apache/druid/query/aggregation/AggregationTestHelper.java index 3bc3ded22012..83b25e0d9681 100644 --- a/processing/src/test/java/org/apache/druid/query/aggregation/AggregationTestHelper.java +++ b/processing/src/test/java/org/apache/druid/query/aggregation/AggregationTestHelper.java @@ -194,15 +194,13 @@ public static AggregationTestHelper createSelectQueryAggregationTestHelper( SelectQueryQueryToolChest toolchest = new SelectQueryQueryToolChest( TestHelper.makeJsonMapper(), - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - configSupplier + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ); SelectQueryRunnerFactory factory = new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( TestHelper.makeJsonMapper(), - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - configSupplier + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ), new SelectQueryEngine( ), diff --git a/processing/src/test/java/org/apache/druid/query/search/SearchBinaryFnTest.java b/processing/src/test/java/org/apache/druid/query/search/SearchBinaryFnTest.java index ea566c46fe25..2e5ce9d0ba10 100644 --- a/processing/src/test/java/org/apache/druid/query/search/SearchBinaryFnTest.java +++ b/processing/src/test/java/org/apache/druid/query/search/SearchBinaryFnTest.java @@ -306,7 +306,7 @@ private List toHits(Comparator comparator, String... hits) List result = new ArrayList<>(); for (String hit : hits) { int index = hit.indexOf(':'); - result.add(new SearchHit(hit.substring(0, index), hit.substring(index + 1))); + result.add(new SearchHit(hit.substring(0, index), hit.substring(index + 1))); //-V6009 } Collections.sort(result, comparator); return result; diff --git a/processing/src/test/java/org/apache/druid/query/select/MultiSegmentSelectQueryTest.java b/processing/src/test/java/org/apache/druid/query/select/MultiSegmentSelectQueryTest.java index 1b7eacc449f0..d338b9e4a40d 100644 --- a/processing/src/test/java/org/apache/druid/query/select/MultiSegmentSelectQueryTest.java +++ b/processing/src/test/java/org/apache/druid/query/select/MultiSegmentSelectQueryTest.java @@ -75,8 +75,7 @@ public class MultiSegmentSelectQueryTest private static final SelectQueryQueryToolChest toolChest = new SelectQueryQueryToolChest( new DefaultObjectMapper(), - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - configSupplier + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ); private static final QueryRunnerFactory factory = new SelectQueryRunnerFactory( diff --git a/processing/src/test/java/org/apache/druid/query/select/SelectQueryQueryToolChestTest.java b/processing/src/test/java/org/apache/druid/query/select/SelectQueryQueryToolChestTest.java index 46f12daad0ff..2f8cfd6fbabe 100644 --- a/processing/src/test/java/org/apache/druid/query/select/SelectQueryQueryToolChestTest.java +++ b/processing/src/test/java/org/apache/druid/query/select/SelectQueryQueryToolChestTest.java @@ -38,8 +38,7 @@ public class SelectQueryQueryToolChestTest private static final SelectQueryQueryToolChest toolChest = new SelectQueryQueryToolChest( new DefaultObjectMapper(), - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - configSupplier + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ); @Test diff --git a/processing/src/test/java/org/apache/druid/query/select/SelectQueryRunnerTest.java b/processing/src/test/java/org/apache/druid/query/select/SelectQueryRunnerTest.java index 3034315c5984..55db5ef4e0d8 100644 --- a/processing/src/test/java/org/apache/druid/query/select/SelectQueryRunnerTest.java +++ b/processing/src/test/java/org/apache/druid/query/select/SelectQueryRunnerTest.java @@ -129,8 +129,7 @@ public class SelectQueryRunnerTest private static final SelectQueryQueryToolChest toolChest = new SelectQueryQueryToolChest( new DefaultObjectMapper(), - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - configSupplier + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ); @Parameterized.Parameters(name = "{0}:descending={1}") diff --git a/processing/src/test/java/org/apache/druid/segment/TestHelper.java b/processing/src/test/java/org/apache/druid/segment/TestHelper.java index 19741e3bcebc..3450e46407a2 100644 --- a/processing/src/test/java/org/apache/druid/segment/TestHelper.java +++ b/processing/src/test/java/org/apache/druid/segment/TestHelper.java @@ -316,16 +316,12 @@ private static void assertRow(String msg, Row expected, Row actual) final Object actualValue = actualMap.get(key); if (expectedValue instanceof Float || expectedValue instanceof Double) { - if (expectedValue == null) { - Assert.assertNull(actualValue); - } else { - Assert.assertEquals( - StringUtils.format("%s: key[%s]", msg, key), - ((Number) expectedValue).doubleValue(), - ((Number) actualValue).doubleValue(), - Math.abs(((Number) expectedValue).doubleValue() * 1e-6) - ); - } + Assert.assertEquals( + StringUtils.format("%s: key[%s]", msg, key), + ((Number) expectedValue).doubleValue(), + ((Number) actualValue).doubleValue(), + Math.abs(((Number) expectedValue).doubleValue() * 1e-6) + ); } else { Assert.assertEquals( StringUtils.format("%s: key[%s]", msg, key), diff --git a/server/src/main/java/org/apache/druid/client/selector/ServerSelector.java b/server/src/main/java/org/apache/druid/client/selector/ServerSelector.java index a485dbaa955c..ac4fb84b477d 100644 --- a/server/src/main/java/org/apache/druid/client/selector/ServerSelector.java +++ b/server/src/main/java/org/apache/druid/client/selector/ServerSelector.java @@ -127,7 +127,7 @@ public List getCandidates(final int numCandidates) .map(server -> server.getServer().getMetadata()) .forEach(candidates::add); - if (candidates.size() < numCandidates) { + if (candidates.size() < numCandidates) { //-V6007: false alarm due to a bug in PVS-Studio strategy.pick(realtimeServers, segment.get(), numCandidates - candidates.size()) .stream() .map(server -> server.getServer().getMetadata()) diff --git a/server/src/main/java/org/apache/druid/server/http/HostAndPortWithScheme.java b/server/src/main/java/org/apache/druid/server/http/HostAndPortWithScheme.java index 428a51d14018..d645697a5631 100644 --- a/server/src/main/java/org/apache/druid/server/http/HostAndPortWithScheme.java +++ b/server/src/main/java/org/apache/druid/server/http/HostAndPortWithScheme.java @@ -43,7 +43,7 @@ public static HostAndPortWithScheme fromString(String hostPortMaybeSchemeString) { if (hostPortMaybeSchemeString.startsWith("http")) { return HostAndPortWithScheme.fromString( - hostPortMaybeSchemeString.substring(0, hostPortMaybeSchemeString.indexOf(':')), + hostPortMaybeSchemeString.substring(0, hostPortMaybeSchemeString.indexOf(':')), //-V6009 hostPortMaybeSchemeString.substring(hostPortMaybeSchemeString.indexOf(':') + 1) ); } diff --git a/server/src/test/java/org/apache/druid/client/CachingClusteredClientTest.java b/server/src/test/java/org/apache/druid/client/CachingClusteredClientTest.java index 59367c657164..de24926a3ab9 100644 --- a/server/src/test/java/org/apache/druid/client/CachingClusteredClientTest.java +++ b/server/src/test/java/org/apache/druid/client/CachingClusteredClientTest.java @@ -1315,8 +1315,7 @@ public void testSelectCaching() getDefaultQueryRunner(), new SelectQueryQueryToolChest( JSON_MAPPER, - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - SELECT_CONFIG_SUPPLIER + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ) ); HashMap context = new HashMap(); @@ -1393,8 +1392,7 @@ public void testSelectCachingRenamedOutputName() getDefaultQueryRunner(), new SelectQueryQueryToolChest( JSON_MAPPER, - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - SELECT_CONFIG_SUPPLIER + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ) ); HashMap context = new HashMap(); diff --git a/server/src/test/java/org/apache/druid/client/CachingClusteredClientTestUtils.java b/server/src/test/java/org/apache/druid/client/CachingClusteredClientTestUtils.java index d0571c79c8fd..646031db7f9a 100644 --- a/server/src/test/java/org/apache/druid/client/CachingClusteredClientTestUtils.java +++ b/server/src/test/java/org/apache/druid/client/CachingClusteredClientTestUtils.java @@ -92,8 +92,7 @@ public static Pair createWarehouse( SelectQuery.class, new SelectQueryQueryToolChest( objectMapper, - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - selectConfigSupplier + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ) ) .put( diff --git a/server/src/test/java/org/apache/druid/client/cache/ByteCountingLRUMapTest.java b/server/src/test/java/org/apache/druid/client/cache/ByteCountingLRUMapTest.java index de8285e70b5c..e5c62db43703 100644 --- a/server/src/test/java/org/apache/druid/client/cache/ByteCountingLRUMapTest.java +++ b/server/src/test/java/org/apache/druid/client/cache/ByteCountingLRUMapTest.java @@ -92,10 +92,10 @@ public void testSameKeyUpdate() final ByteBuffer k = ByteBuffer.allocate(1); assertMapValues(0, 0, 0); - map.put(k, new byte[1]); - map.put(k, new byte[2]); - map.put(k, new byte[5]); - map.put(k, new byte[3]); + map.put(k, new byte[1]); //-V6033: suppress "An item with the same key has already been added" + map.put(k, new byte[2]); //-V6033 + map.put(k, new byte[5]); //-V6033 + map.put(k, new byte[3]); //-V6033 assertMapValues(1, 4, 0); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java b/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java index 3b2223514f59..53d6d34e7d39 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java @@ -160,10 +160,7 @@ public void setUp() throws Exception null, 10, null, - false, - false, - new Duration("PT0s"), - Duration.millis(10) + new Duration("PT0s") ); sourceLoadQueueChildrenCache = new PathChildrenCache( curator, diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java index 793bd287b950..5ff127504957 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java @@ -137,10 +137,7 @@ public void setUp() throws Exception null, 10, null, - false, - false, - new Duration("PT0s"), - Duration.millis(10) + new Duration("PT0s") ); pathChildrenCache = new PathChildrenCache( curator, diff --git a/server/src/test/java/org/apache/druid/server/coordinator/HttpLoadQueuePeonTest.java b/server/src/test/java/org/apache/druid/server/coordinator/HttpLoadQueuePeonTest.java index 894472a2cbcc..f45348d5ef67 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/HttpLoadQueuePeonTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/HttpLoadQueuePeonTest.java @@ -82,10 +82,7 @@ public class HttpLoadQueuePeonTest null, 10, null, - false, - false, - Duration.ZERO, - Duration.millis(10) + Duration.ZERO ) { @Override diff --git a/server/src/test/java/org/apache/druid/server/coordinator/LoadQueuePeonTest.java b/server/src/test/java/org/apache/druid/server/coordinator/LoadQueuePeonTest.java index 8d8271dab1dc..3da088800a80 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/LoadQueuePeonTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/LoadQueuePeonTest.java @@ -97,10 +97,7 @@ public void testMultipleLoadDropSegments() throws Exception null, 10, null, - false, - false, - Duration.millis(0), - Duration.millis(10) + Duration.millis(0) ) ); @@ -295,10 +292,7 @@ public void testFailAssign() throws Exception null, 10, null, - false, - false, - new Duration("PT1s"), - Duration.millis(10) + new Duration("PT1s") ) ); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/LoadQueuePeonTester.java b/server/src/test/java/org/apache/druid/server/coordinator/LoadQueuePeonTester.java index c979671ff847..d71e903bfccf 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/LoadQueuePeonTester.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/LoadQueuePeonTester.java @@ -46,10 +46,7 @@ public LoadQueuePeonTester() null, 10, null, - false, - false, - new Duration("PT1s"), - Duration.millis(10) + new Duration("PT1s") ) ); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/TestDruidCoordinatorConfig.java b/server/src/test/java/org/apache/druid/server/coordinator/TestDruidCoordinatorConfig.java index e1b91354de70..03d8650493b2 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/TestDruidCoordinatorConfig.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/TestDruidCoordinatorConfig.java @@ -44,10 +44,7 @@ public TestDruidCoordinatorConfig( Duration coordinatorKillDurationToRetain, int coordinatorKillMaxSegments, String consoleStatic, - boolean mergeSegments, - boolean convertSegments, - Duration getLoadQueuePeonRepeatDelay, - Duration CuratorCreateZkNodesRepeatDelay + Duration getLoadQueuePeonRepeatDelay ) { this.coordinatorStartDelay = coordinatorStartDelay; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/helper/DruidCoordinatorSegmentKillerTest.java b/server/src/test/java/org/apache/druid/server/coordinator/helper/DruidCoordinatorSegmentKillerTest.java index 7ee58a2a8f19..0f0bc033cb93 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/helper/DruidCoordinatorSegmentKillerTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/helper/DruidCoordinatorSegmentKillerTest.java @@ -111,10 +111,7 @@ private void testFindIntervalForKillTask(List segmentManagerResult, In Duration.parse("PT86400S"), 1000, null, - false, - false, - Duration.ZERO, - Duration.millis(10) + Duration.ZERO ) ); diff --git a/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java b/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java index c3de4c883fb7..4a17bf36d830 100644 --- a/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java +++ b/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java @@ -119,8 +119,8 @@ public void testResourcesFilteringNoAccess() Assert.fail(); } catch (ForbiddenException e) { + EasyMock.verify(req, request, authorizerMapper); throw e; } - EasyMock.verify(req, request, authorizerMapper); } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java index 590205ea28f0..cc4e935c311a 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java @@ -516,8 +516,7 @@ public int getNumMergeBuffers() new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( TestHelper.makeJsonMapper(), - QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator(), - SELECT_CONFIG_SUPPLIER + QueryRunnerTestHelper.noopIntervalChunkingQueryRunnerDecorator() ), new SelectQueryEngine(), QueryRunnerTestHelper.NOOP_QUERYWATCHER From c7435c1594faf26b4c877eb4ed511bc52f233db7 Mon Sep 17 00:00:00 2001 From: Roman Leventov Date: Wed, 29 May 2019 09:29:15 +0200 Subject: [PATCH 2/2] Address comments --- .../druid/segment/loading/DataSegmentKiller.java | 10 ++++++++++ ...rBasicAuthenticatorMetadataStorageUpdater.java | 3 ++- .../storage/google/GoogleDataSegmentKiller.java | 2 +- .../google/GoogleDataSegmentKillerTest.java | 4 ++-- .../histogram/ApproximateHistogram.java | 2 +- .../druid/storage/s3/S3DataSegmentKiller.java | 7 +------ .../overlord/hrtr/HttpRemoteTaskRunnerTest.java | 15 +++++++++++++-- .../clients/CoordinatorResourceTestClient.java | 6 ------ .../druid/query/search/SearchBinaryFnTest.java | 8 ++++++-- .../druid/server/http/HostAndPortWithScheme.java | 9 +++++++-- 10 files changed, 43 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/org/apache/druid/segment/loading/DataSegmentKiller.java b/core/src/main/java/org/apache/druid/segment/loading/DataSegmentKiller.java index 20af38890a9a..36c561269f0e 100644 --- a/core/src/main/java/org/apache/druid/segment/loading/DataSegmentKiller.java +++ b/core/src/main/java/org/apache/druid/segment/loading/DataSegmentKiller.java @@ -20,6 +20,7 @@ package org.apache.druid.segment.loading; import org.apache.druid.guice.annotations.ExtensionPoint; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.DataSegment; @@ -30,6 +31,15 @@ public interface DataSegmentKiller { Logger log = new Logger(DataSegmentKiller.class); + static String descriptorPath(String path) + { + int lastPathSeparatorIndex = path.lastIndexOf('/'); + if (lastPathSeparatorIndex == -1) { + throw new IAE("Invalid path: [%s], should contain '/'", path); + } + return path.substring(0, lastPathSeparatorIndex) + "/descriptor.json"; + } + /** * Removes segment files (index and metadata) from deep storage. * @param segment the segment to kill diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java index 49a1ab332370..ab6bb8ed292b 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java @@ -91,7 +91,8 @@ public CoordinatorBasicAuthenticatorMetadataStorageUpdater( BasicAuthCommonCacheConfig commonCacheConfig, @Smile ObjectMapper objectMapper, BasicAuthenticatorCacheNotifier cacheNotifier, - ConfigManager configManager // -V6022: ConfigManager creates the db table we need, set a dependency here + ConfigManager configManager // -V6022 (unused parameter): ConfigManager creates the db table we need, + // set a dependency here ) { this.exec = Execs.scheduledSingleThreaded("CoordinatorBasicAuthenticatorMetadataStorageUpdater-Exec--%d"); diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java index 88ad394c370a..27cb989c1ad0 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java @@ -52,7 +52,7 @@ public void kill(DataSegment segment) throws SegmentLoadingException Map loadSpec = segment.getLoadSpec(); final String bucket = MapUtils.getString(loadSpec, "bucket"); final String indexPath = MapUtils.getString(loadSpec, "path"); - final String descriptorPath = indexPath.substring(0, indexPath.lastIndexOf('/')) + "/descriptor.json"; //-V6009 + final String descriptorPath = DataSegmentKiller.descriptorPath(indexPath); try { deleteIfPresent(bucket, indexPath); diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java index b65285655449..7b0812b37d39 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java @@ -24,6 +24,7 @@ import com.google.api.client.json.jackson2.JacksonFactory; import com.google.common.collect.ImmutableMap; import org.apache.druid.java.util.common.Intervals; +import org.apache.druid.segment.loading.DataSegmentKiller; import org.apache.druid.segment.loading.SegmentLoadingException; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.partition.NoneShardSpec; @@ -40,8 +41,7 @@ public class GoogleDataSegmentKillerTest extends EasyMockSupport { private static final String bucket = "bucket"; private static final String indexPath = "test/2015-04-12T00:00:00.000Z_2015-04-13T00:00:00.000Z/1/0/index.zip"; - private static final String descriptorPath = - indexPath.substring(0, indexPath.lastIndexOf('/')) + "/descriptor.json"; //-V6009 + private static final String descriptorPath = DataSegmentKiller.descriptorPath(indexPath); private static final DataSegment dataSegment = new DataSegment( "test", diff --git a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java index 57fd51a53b87..eeac3a53af6c 100644 --- a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java +++ b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java @@ -419,7 +419,7 @@ protected void mergeInsert(final int mergeAt, int insertAt, final float v, final // use unused slot to shift array left or right and make space for the new bin to insert if (insertAt < unusedIndex) { shiftRight(insertAt, unusedIndex); - } else { // insertAt >= unusedIndex + } else { shiftLeft(unusedIndex, insertAt - 1); insertAt--; } diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java index cb2d6ed4ac98..6df4161d846b 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java @@ -50,7 +50,7 @@ public void kill(DataSegment segment) throws SegmentLoadingException Map loadSpec = segment.getLoadSpec(); String s3Bucket = MapUtils.getString(loadSpec, "bucket"); String s3Path = MapUtils.getString(loadSpec, "key"); - String s3DescriptorPath = descriptorPathForSegmentPath(s3Path); + String s3DescriptorPath = DataSegmentKiller.descriptorPath(s3Path); if (s3Client.doesObjectExist(s3Bucket, s3Path)) { log.info("Removing index file[s3://%s/%s] from s3!", s3Bucket, s3Path); @@ -68,11 +68,6 @@ public void kill(DataSegment segment) throws SegmentLoadingException } } - private static String descriptorPathForSegmentPath(String s3Path) - { - return s3Path.substring(0, s3Path.lastIndexOf('/')) + "/descriptor.json"; //-V6009 - } - @Override public void killAll() { diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java index 2a97b5906499..5eddb2e9e5bb 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java @@ -46,6 +46,7 @@ import org.apache.druid.indexing.overlord.setup.DefaultWorkerBehaviorConfig; import org.apache.druid.indexing.worker.TaskAnnouncement; import org.apache.druid.indexing.worker.Worker; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.concurrent.Execs; import org.apache.druid.java.util.http.client.HttpClient; @@ -1285,8 +1286,18 @@ private static WorkerHolder createWorkerHolder( { return new WorkerHolder(smileMapper, httpClient, config, workersSyncExec, listener, worker) { - private final String workerHost = worker.getHost().substring(0, worker.getHost().indexOf(':')); //-V6009 - private final int workerPort = Integer.parseInt(worker.getHost().substring(worker.getHost().indexOf(':') + 1)); + private final String workerHost; + private final int workerPort; + + { + String hostAndPort = worker.getHost(); + int colonIndex = hostAndPort.indexOf(':'); + if (colonIndex == -1) { + throw new IAE("Invalid host and port: [%s]", colonIndex); + } + workerHost = hostAndPort.substring(0, colonIndex); + workerPort = Integer.parseInt(hostAndPort.substring(colonIndex + 1)); + } @Override public void start() diff --git a/integration-tests/src/main/java/org/apache/druid/testing/clients/CoordinatorResourceTestClient.java b/integration-tests/src/main/java/org/apache/druid/testing/clients/CoordinatorResourceTestClient.java index 7b73efb09791..678169a640a1 100644 --- a/integration-tests/src/main/java/org/apache/druid/testing/clients/CoordinatorResourceTestClient.java +++ b/integration-tests/src/main/java/org/apache/druid/testing/clients/CoordinatorResourceTestClient.java @@ -241,12 +241,6 @@ public Map initializeLookups(String filePath) throws Exception ); } - @SuppressWarnings("unused") // It's unclear whether this call to readValue() has desirable side effects. - Map results = jsonMapper.readValue( - response.getContent(), - new TypeReference>(){} - ); - StatusResponseHolder response2 = httpClient.go( new Request(HttpMethod.POST, new URL(url)).setContent( "application/json", diff --git a/processing/src/test/java/org/apache/druid/query/search/SearchBinaryFnTest.java b/processing/src/test/java/org/apache/druid/query/search/SearchBinaryFnTest.java index 2e5ce9d0ba10..a48fc66506e2 100644 --- a/processing/src/test/java/org/apache/druid/query/search/SearchBinaryFnTest.java +++ b/processing/src/test/java/org/apache/druid/query/search/SearchBinaryFnTest.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList; import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.query.Result; import org.apache.druid.query.ordering.StringComparators; @@ -305,8 +306,11 @@ private List toHits(Comparator comparator, String... hits) { List result = new ArrayList<>(); for (String hit : hits) { - int index = hit.indexOf(':'); - result.add(new SearchHit(hit.substring(0, index), hit.substring(index + 1))); //-V6009 + int colonIndex = hit.indexOf(':'); + if (colonIndex == -1) { + throw new IAE("Invalid hit: [%s]", hit); + } + result.add(new SearchHit(hit.substring(0, colonIndex), hit.substring(colonIndex + 1))); } Collections.sort(result, comparator); return result; diff --git a/server/src/main/java/org/apache/druid/server/http/HostAndPortWithScheme.java b/server/src/main/java/org/apache/druid/server/http/HostAndPortWithScheme.java index d645697a5631..0c584aab8d0a 100644 --- a/server/src/main/java/org/apache/druid/server/http/HostAndPortWithScheme.java +++ b/server/src/main/java/org/apache/druid/server/http/HostAndPortWithScheme.java @@ -21,6 +21,7 @@ import com.google.common.base.Preconditions; import com.google.common.net.HostAndPort; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.StringUtils; public class HostAndPortWithScheme @@ -42,9 +43,13 @@ public static HostAndPortWithScheme fromParts(String scheme, String host, int po public static HostAndPortWithScheme fromString(String hostPortMaybeSchemeString) { if (hostPortMaybeSchemeString.startsWith("http")) { + int colonIndex = hostPortMaybeSchemeString.indexOf(':'); + if (colonIndex == -1) { + throw new IAE("Invalid host with scheme string: [%s]", hostPortMaybeSchemeString); + } return HostAndPortWithScheme.fromString( - hostPortMaybeSchemeString.substring(0, hostPortMaybeSchemeString.indexOf(':')), //-V6009 - hostPortMaybeSchemeString.substring(hostPortMaybeSchemeString.indexOf(':') + 1) + hostPortMaybeSchemeString.substring(0, colonIndex), + hostPortMaybeSchemeString.substring(colonIndex + 1) ); } return HostAndPortWithScheme.fromString("http", hostPortMaybeSchemeString);