github-code-scanning[bot] commented on code in PR #14239:
URL: https://github.com/apache/druid/pull/14239#discussion_r1189456536
##########
server/src/test/java/org/apache/druid/server/log/LoggingRequestLoggerProviderTest.java:
##########
@@ -77,7 +77,7 @@
final Properties properties = new Properties();
properties.put(propertyPrefix + ".type", "noop");
provider.inject(properties, injector.getInstance(JsonConfigurator.class));
- Assert.assertThat(provider.get().get().get(),
Matchers.instanceOf(NoopRequestLogger.class));
+ Assert.assertThat(provider.get().get(),
Matchers.instanceOf(NoopRequestLogger.class));
Review Comment:
## Deprecated method or constructor invocation
Invoking [Assert.assertThat](1) should be avoided because it has been
deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/4934)
##########
server/src/test/java/org/apache/druid/client/cache/CacheConfigTest.java:
##########
@@ -126,7 +126,7 @@
{
properties.put(PROPERTY_PREFIX + ".numBackgroundThreads", "BABBA YAGA");
configProvider.inject(properties, configurator);
- CacheConfig config = configProvider.get().get();
+ CacheConfig config = configProvider.get();
Review Comment:
## Unread local variable
Variable 'CacheConfig config' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/4935)
##########
server/src/test/java/org/apache/druid/client/cache/CacheConfigTest.java:
##########
@@ -154,7 +154,7 @@
{
properties.put(PROPERTY_PREFIX + ".populateCache", "FaLse");
configProvider.inject(properties, configurator);
- CacheConfig config = configProvider.get().get();
+ CacheConfig config = configProvider.get();
Review Comment:
## Unread local variable
Variable 'CacheConfig config' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/4938)
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/ThreadingTaskRunner.java:
##########
@@ -167,7 +167,7 @@
);
}
- final File taskDir = new File(baseDirForTask,
task.getId());
+ final File taskDir = new
File(storageSlot.getDirectory(), task.getId());
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/druid/security/code-scanning/4940)
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java:
##########
@@ -154,20 +153,19 @@
@Override
public TaskStatus call()
{
-
- final File baseDirForTask;
+ final TaskStorageDirTracker.StorageSlot storageSlot;
try {
- baseDirForTask = getTracker().pickBaseDir(task.getId());
+ storageSlot = getTracker().pickStorageSlot(task.getId());
}
catch (RuntimeException e) {
- LOG.error(e, "Failed to get directory for task [%s],
cannot schedule.", task.getId());
+ LOG.warn(e, "Failed to get storage slot for task [%s],
cannot schedule.", task.getId());
return TaskStatus.failure(
task.getId(),
- StringUtils.format("Could not schedule due to error
[%s]", e.getMessage())
+ StringUtils.format("Failed to get storage slot due to
error [%s]", e.getMessage())
);
}
- final File taskDir = new File(baseDirForTask, task.getId());
+ final File taskDir = new File(storageSlot.getDirectory(),
task.getId());
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/druid/security/code-scanning/4939)
##########
server/src/test/java/org/apache/druid/client/cache/CacheConfigTest.java:
##########
@@ -144,7 +144,7 @@
{
properties.put(PROPERTY_PREFIX + ".populateCache", "FALSE");
configProvider.inject(properties, configurator);
- CacheConfig config = configProvider.get().get();
+ CacheConfig config = configProvider.get();
Review Comment:
## Unread local variable
Variable 'CacheConfig config' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/4937)
##########
server/src/test/java/org/apache/druid/client/cache/CacheConfigTest.java:
##########
@@ -135,7 +135,7 @@
{
properties.put(PROPERTY_PREFIX + ".populateCache", "TRUE");
configProvider.inject(properties, configurator);
- CacheConfig config = configProvider.get().get();
+ CacheConfig config = configProvider.get();
Review Comment:
## Unread local variable
Variable 'CacheConfig config' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/4936)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/TaskStorageDirTracker.java:
##########
@@ -24,91 +24,202 @@
import org.apache.druid.indexing.worker.config.WorkerConfig;
import org.apache.druid.java.util.common.FileUtils;
import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.lifecycle.LifecycleStart;
import javax.annotation.Nullable;
import java.io.File;
import java.io.IOException;
+import java.util.Arrays;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
+/**
+ * Used to pick storage slots for tasks when run from the middle manager.
+ */
public class TaskStorageDirTracker
{
public static TaskStorageDirTracker fromConfigs(WorkerConfig workerConfig,
TaskConfig taskConfig)
{
+ final List<File> baseTaskDirs;
if (workerConfig == null) {
- return new
TaskStorageDirTracker(ImmutableList.of(taskConfig.getBaseTaskDir()));
+ baseTaskDirs = ImmutableList.of(taskConfig.getBaseTaskDir());
} else {
final List<String> basePaths = workerConfig.getBaseTaskDirs();
if (basePaths == null) {
- return new
TaskStorageDirTracker(ImmutableList.of(taskConfig.getBaseTaskDir()));
+ baseTaskDirs = ImmutableList.of(taskConfig.getBaseTaskDir());
+ } else {
+ baseTaskDirs =
basePaths.stream().map(File::new).collect(Collectors.toList());
}
- return new TaskStorageDirTracker(
- basePaths.stream().map(File::new).collect(Collectors.toList())
- );
}
+
+ return fromBaseDirs(baseTaskDirs, workerConfig.getCapacity(),
workerConfig.getBaseTaskDirSize());
Review Comment:
## Dereferenced variable may be null
Variable [workerConfig](1) may be null at this access as suggested by
[this](2) null guard.
[Show more
details](https://github.com/apache/druid/security/code-scanning/4941)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
