maytasm commented on a change in pull request #10479:
URL: https://github.com/apache/druid/pull/10479#discussion_r499908547
##########
File path:
server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java
##########
@@ -180,24 +183,43 @@ public DruidCoordinatorRuntimeParams
run(DruidCoordinatorRuntimeParams params)
}
/**
- * Each compaction task can run a parallel indexing task. When we count the
number of current running
- * compaction tasks, we should count the sub tasks of the parallel indexing
task as well. However, we currently
- * don't have a good way to get the number of current running sub tasks
except poking each supervisor task,
- * which is complex to handle all kinds of failures. Here, we simply return
{@code maxNumConcurrentSubTasks} instead
- * to estimate the number of sub tasks conservatively. This should be ok
since it won't affect to the performance of
- * other ingestion types.
+ * Returns the maximum number of task slots used by one compaction task at
any time when the task is issued with
+ * the given tuningConfig.
*/
- private int findNumMaxConcurrentSubTasks(@Nullable
ClientCompactionTaskQueryTuningConfig tuningConfig)
+ @VisibleForTesting
+ static int findMaxNumTaskSlotsUsedByOneCompactionTask(@Nullable
ClientCompactionTaskQueryTuningConfig tuningConfig)
{
- if (tuningConfig != null && tuningConfig.getMaxNumConcurrentSubTasks() !=
null) {
- // The actual number of subtasks might be smaller than the configured
max.
- // However, we use the max to simplify the estimation here.
- return tuningConfig.getMaxNumConcurrentSubTasks();
+ if (isParallelMode(tuningConfig)) {
+ @Nullable Integer maxNumConcurrentSubTasks =
tuningConfig.getMaxNumConcurrentSubTasks();
+ // Max number of task slots used in parallel mode =
maxNumConcurrentSubTasks + 1 (supervisor task)
+ return (maxNumConcurrentSubTasks == null ? 1 : maxNumConcurrentSubTasks)
+ 1;
} else {
- return 0;
+ return 1;
}
}
+ /**
+ * Returns true if the compaction task can run in the parallel mode with the
given tuningConfig.
+ * This method should be synchronized with
ParallelIndexSupervisorTask.isParallelMode(InputSource,
ParallelIndexTuningConfig).
+ */
+ @VisibleForTesting
+ static boolean isParallelMode(@Nullable
ClientCompactionTaskQueryTuningConfig tuningConfig)
Review comment:
nit: can we add comments on both this method and the one in
ParallelIndexSupervisorTask linking back to each other. Hopefully, this will
make maintaining a little easier. i.e. if someone changed the method in
ParallelIndexSupervisorTask, they can be aware of this version in
CompactSegments.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
