Skip to content

Commit 99186c6

Browse files
committed
Address comments
1 parent 8f15808 commit 99186c6

File tree

2 files changed

+11
-9
lines changed

2 files changed

+11
-9
lines changed

pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ public class PinotHelixTaskResourceManager {
8484
private static final String TASK_QUEUE_PREFIX = "TaskQueue" + TASK_NAME_SEPARATOR;
8585
private static final String TASK_PREFIX = "Task" + TASK_NAME_SEPARATOR;
8686
private static final String UNKNOWN_TABLE_NAME = "unknown";
87+
private static final String UNKNOWN_TENANT_NAME = "unknown";
8788

8889
private final TaskDriver _taskDriver;
8990
private final PinotHelixResourceManager _helixResourceManager;
@@ -968,19 +969,19 @@ private boolean hasTasksForTable(String taskName, String tableNameWithType) {
968969
*/
969970
private String getTenantForTable(String tableName) {
970971
if (tableName == null || UNKNOWN_TABLE_NAME.equals(tableName)) {
971-
return UNKNOWN_TABLE_NAME;
972+
return UNKNOWN_TENANT_NAME;
972973
}
973974

974975
try {
975976
TableConfig tableConfig = _helixResourceManager.getTableConfig(tableName);
976977
if (tableConfig != null && tableConfig.getTenantConfig() != null) {
977978
String serverTenant = tableConfig.getTenantConfig().getServer();
978-
return serverTenant != null ? serverTenant : UNKNOWN_TABLE_NAME;
979+
return serverTenant != null ? serverTenant : UNKNOWN_TENANT_NAME;
979980
}
980-
return UNKNOWN_TABLE_NAME;
981+
return UNKNOWN_TENANT_NAME;
981982
} catch (Exception e) {
982983
LOGGER.warn("Failed to determine tenant for table: {}", tableName, e);
983-
return UNKNOWN_TABLE_NAME;
984+
return UNKNOWN_TENANT_NAME;
984985
}
985986
}
986987

@@ -1017,19 +1018,20 @@ public synchronized TaskSummaryResponse getTasksSummary(@Nullable String tenantF
10171018
String taskName = entry.getKey();
10181019
TaskCount totalTaskCount = entry.getValue();
10191020

1020-
// Skip if this parent task has no running/waiting tasks (optimization: avoid table breakdown call)
1021+
// Skip if this parent task has no running/waiting tasks
10211022
if (totalTaskCount.getRunning() == 0 && totalTaskCount.getWaiting() == 0) {
10221023
continue;
10231024
}
10241025

10251026
// Get the table name from the first subtask
1026-
// Note: In practice, all subtasks in a parent task belong to the same table
1027+
// Note: All subtasks in a parent task belong to the same table
10271028
List<PinotTaskConfig> subtaskConfigs = getSubtaskConfigs(taskName);
10281029
if (subtaskConfigs.isEmpty()) {
10291030
continue;
10301031
}
10311032

1032-
Map<String, String> configs = subtaskConfigs.get(0).getConfigs();
1033+
PinotTaskConfig firstSubtaskConfig = subtaskConfigs.get(0);
1034+
Map<String, String> configs = firstSubtaskConfig.getConfigs();
10331035
String tableName = (configs != null)
10341036
? configs.getOrDefault(MinionConstants.TABLE_NAME_KEY, UNKNOWN_TABLE_NAME)
10351037
: UNKNOWN_TABLE_NAME;

pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManagerTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1358,7 +1358,7 @@ public void testGetTasksSummaryWithSingleTenant() {
13581358
PinotHelixTaskResourceManager.TaskCount taskCount = new PinotHelixTaskResourceManager.TaskCount();
13591359
taskCount.addTaskState(TaskPartitionState.RUNNING);
13601360
taskCount.addTaskState(TaskPartitionState.RUNNING);
1361-
taskCount.addTaskState(null);
1361+
taskCount.addTaskState(null); // null state counts as waiting
13621362
Map<String, PinotHelixTaskResourceManager.TaskCount> taskCounts = new HashMap<>();
13631363
taskCounts.put(taskName, taskCount);
13641364
when(spyMgr.getTaskCounts(taskType)).thenReturn(taskCounts);
@@ -1627,7 +1627,7 @@ public void testGetTasksSummaryWithMultipleTaskTypes() {
16271627

16281628
// Task 2: TaskType2 with waiting tasks
16291629
PinotHelixTaskResourceManager.TaskCount taskCount2 = new PinotHelixTaskResourceManager.TaskCount();
1630-
taskCount2.addTaskState(null);
1630+
taskCount2.addTaskState(null); // null state counts as waiting
16311631
Map<String, PinotHelixTaskResourceManager.TaskCount> taskCounts2 = new HashMap<>();
16321632
taskCounts2.put(taskName2, taskCount2);
16331633
when(spyMgr.getTaskCounts(taskType2)).thenReturn(taskCounts2);

0 commit comments

Comments
 (0)