zhtaoxiang commented on code in PR #12459:
URL: https://github.com/apache/pinot/pull/12459#discussion_r1542088076
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java:
##########
@@ -531,78 +532,98 @@ private synchronized Map<String, String>
scheduleTasks(List<String> tableNamesWi
/**
* Helper method to schedule task with the given task generator for the
given tables that have the task enabled.
- * Returns the task name, or {@code null} if no task is scheduled.
+ * Returns the list of task names, or {@code null} if no task is scheduled.
*/
@Nullable
- private String scheduleTask(PinotTaskGenerator taskGenerator,
List<TableConfig> enabledTableConfigs,
+ private List<String> scheduleTask(PinotTaskGenerator taskGenerator,
List<TableConfig> enabledTableConfigs,
boolean isLeader) {
LOGGER.info("Trying to schedule task type: {}, isLeader: {}",
taskGenerator.getTaskType(), isLeader);
- List<PinotTaskConfig> pinotTaskConfigs;
- try {
- /* TODO taskGenerator may skip generating tasks for some of the tables
being passed to it.
- In that case, we should not be storing success timestamps for those
table. Same with exceptions that should
- only be associated with the table for which it was raised and not
every eligible table. We can have the
- generateTasks() return a list of TaskGeneratorMostRecentRunInfo for
each table
- */
- pinotTaskConfigs = taskGenerator.generateTasks(enabledTableConfigs);
- long successRunTimestamp = System.currentTimeMillis();
- for (TableConfig tableConfig : enabledTableConfigs) {
-
_taskManagerStatusCache.saveTaskGeneratorInfo(tableConfig.getTableName(),
taskGenerator.getTaskType(),
+ Map<String, List<PinotTaskConfig>> minionInstanceTagToTaskConfigs = new
HashMap<>();
+ String taskType = taskGenerator.getTaskType();
+ for (TableConfig tableConfig : enabledTableConfigs) {
+ String tableName = tableConfig.getTableName();
+ try {
+ String minionInstanceTag =
taskGenerator.getMinionInstanceTag(tableConfig);
+ List<PinotTaskConfig> presentTaskConfig =
+ minionInstanceTagToTaskConfigs.computeIfAbsent(minionInstanceTag,
k -> new ArrayList<>());
+ taskGenerator.generateTasks(List.of(tableConfig), presentTaskConfig);
+ minionInstanceTagToTaskConfigs.put(minionInstanceTag,
presentTaskConfig);
Review Comment:
This put is not needed, `computeIfAbsent` will put the newly generated list
into the map
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
