Copilot commented on code in PR #16977:
URL: https://github.com/apache/pinot/pull/16977#discussion_r2413918026
##########
pinot-controller/src/main/resources/app/requests/index.ts:
##########
@@ -209,8 +209,8 @@ export const getTasks = (tableName: string, taskType:
string): Promise<AxiosResp
export const getTaskRuntimeConfig = (taskName: string):
Promise<AxiosResponse<TaskRuntimeConfig>> =>
baseApi.get(`/tasks/task/${taskName}/runtime/config`, { headers: {
...headers, Accept: 'application/json' }});
-export const getTaskDebug = (taskName: string):
Promise<AxiosResponse<OperationResponse>> =>
- baseApi.get(`/tasks/task/${taskName}/debug?verbosity=1`, { headers: {
...headers, Accept: 'application/json' } });
+export const getTaskDebug = (taskName: string, tableName: string):
Promise<AxiosResponse<OperationResponse>> =>
+
baseApi.get(`/tasks/task/${taskName}/debug?verbosity=1&tableName=${tableName}`,
{ headers: { ...headers, Accept: 'application/json' } });
Review Comment:
The tableName parameter is not URL-encoded, which could cause issues if the
table name contains special characters. Also, there's no null check for
tableName, which could result in 'tableName=null' being appended to the URL.
Consider using URLSearchParams or conditionally adding the tableName parameter
only when it's not null.
```suggestion
baseApi.get(`/tasks/task/${taskName}/debug`, {
headers: { ...headers, Accept: 'application/json' },
params: {
verbosity: 1,
...(tableName != null ? { tableName } : {})
}
});
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1003,14 +1017,23 @@ private synchronized TaskDebugInfo
getTaskDebugInfo(WorkflowContext workflowCont
TaskCount subtaskCount = new TaskCount();
for (int partition : partitionSet) {
// First get the partition's state and update the subtaskCount
+ String taskIdForPartition =
jobContext.getTaskIdForPartition(partition);
TaskPartitionState partitionState =
jobContext.getPartitionState(partition);
+ TaskConfig helixTaskConfig =
jobConfig.getTaskConfig(taskIdForPartition);
+ PinotTaskConfig pinotTaskConfig = null;
+ if (helixTaskConfig != null) {
+ pinotTaskConfig =
PinotTaskConfig.fromHelixTaskConfig(helixTaskConfig);
+ if ((tableNameWithType != null) &&
(!tableNameWithType.equals(pinotTaskConfig.getTableName()))) {
+ // Filter task configs that match this table name
+ continue;
+ }
+ }
subtaskCount.addTaskState(partitionState);
Review Comment:
The subtaskCount.addTaskState(partitionState) is called after the table
filtering continue statement, which means filtered out tasks won't be counted.
This will result in incorrect subtask counts when table filtering is applied.
Move line 1031 before the filtering logic (after line 1021) to ensure all
partition states are counted regardless of table filtering.
--
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]
