tibrewalpratik17 commented on code in PR #12459:
URL: https://github.com/apache/pinot/pull/12459#discussion_r1546746797


##########
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:
   We are appending to the list we get from "computeIfAbsent" and putting it 
back.



-- 
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

Reply via email to