imply-cheddar commented on code in PR #14239:
URL: https://github.com/apache/druid/pull/14239#discussion_r1192095295
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java:
##########
@@ -304,82 +290,48 @@ public TaskStatus call()
if (context != null) {
for (String propName : context.keySet()) {
if (propName.startsWith(CHILD_PROPERTY_PREFIX)) {
- command.add(
- StringUtils.format(
- "-D%s=%s",
+ command.addSystemProperty(
propName.substring(CHILD_PROPERTY_PREFIX.length()),
task.getContextValue(propName)
- )
);
}
}
}
// add the attemptId as a system property
- command.add(
- StringUtils.format(
- "-D%s=%s",
- "attemptId",
- "1"
- )
- );
+ command.addSystemProperty("attemptId", "1");
// Add dataSource, taskId and taskType for metrics or
logging
- command.add(
- StringUtils.format(
- "-D%s%s=%s",
- MonitorsConfig.METRIC_DIMENSION_PREFIX,
- DruidMetrics.DATASOURCE,
+ command.addSystemProperty(
+ MonitorsConfig.METRIC_DIMENSION_PREFIX +
DruidMetrics.DATASOURCE,
task.getDataSource()
- )
);
- command.add(
- StringUtils.format(
- "-D%s%s=%s",
- MonitorsConfig.METRIC_DIMENSION_PREFIX,
- DruidMetrics.TASK_ID,
+ command.addSystemProperty(
+ MonitorsConfig.METRIC_DIMENSION_PREFIX +
DruidMetrics.TASK_ID,
task.getId()
- )
);
- command.add(
- StringUtils.format(
- "-D%s%s=%s",
- MonitorsConfig.METRIC_DIMENSION_PREFIX,
- DruidMetrics.TASK_TYPE,
+ command.addSystemProperty(
+ MonitorsConfig.METRIC_DIMENSION_PREFIX +
DruidMetrics.TASK_TYPE,
task.getType()
- )
);
- command.add(StringUtils.format("-Ddruid.host=%s",
childHost));
-
command.add(StringUtils.format("-Ddruid.plaintextPort=%d", childPort));
- command.add(StringUtils.format("-Ddruid.tlsPort=%d",
tlsChildPort));
+
+ command.addSystemProperty("druid.host", childHost);
+ command.addSystemProperty("druid.plaintextPort",
childPort);
+ command.addSystemProperty("druid.tlsPort",
tlsChildPort);
// Let tasks know where they are running on.
// This information is used in native parallel
indexing with shuffle.
-
command.add(StringUtils.format("-Ddruid.task.executor.service=%s",
node.getServiceName()));
-
command.add(StringUtils.format("-Ddruid.task.executor.host=%s",
node.getHost()));
- command.add(
-
StringUtils.format("-Ddruid.task.executor.plaintextPort=%d",
node.getPlaintextPort())
- );
- command.add(
- StringUtils.format(
- "-Ddruid.task.executor.enablePlaintextPort=%s",
- node.isEnablePlaintextPort()
- )
- );
-
command.add(StringUtils.format("-Ddruid.task.executor.tlsPort=%d",
node.getTlsPort()));
- command.add(
-
StringUtils.format("-Ddruid.task.executor.enableTlsPort=%s",
node.isEnableTlsPort())
- );
-
command.add(StringUtils.format("-Dlog4j2.configurationFactory=%s",
ConsoleLoggingEnforcementConfigurationFactory.class.getName()));
+
command.addSystemProperty("druid.task.executor.service", node.getServiceName());
+ command.addSystemProperty("druid.task.executor.host",
node.getHost());
+
command.addSystemProperty("druid.task.executor.plaintextPort",
node.getPlaintextPort());
+
command.addSystemProperty("druid.task.executor.enablePlaintextPort",
node.isEnablePlaintextPort());
+
command.addSystemProperty("druid.task.executor.tlsPort", node.getTlsPort());
+
command.addSystemProperty("druid.task.executor.enableTlsPort",
node.isEnableTlsPort());
+
command.addSystemProperty("log4j2.configurationFactory",
ConsoleLoggingEnforcementConfigurationFactory.class.getName());
- // These are not enabled per default to allow the user
to either set or not set them
- // Users are highly suggested to be set in
druid.indexer.runner.javaOpts
- // See
org.apache.druid.concurrent.TaskThreadPriority#getThreadPriorityFromTaskPriority(int)
- // for more information
- // command.add("-XX:+UseThreadPriorities");
- // command.add("-XX:ThreadPriorityPolicy=42");
Review Comment:
They were commented out. commented out code is pointless, it just generates
questions of "is this necessary" and then sits there from release to release
because nobody knows why it's there or when it can be uncommented. I cleaned
it up. If it's necessary, they can be added and not commented out.
--
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]
