Copilot commented on code in PR #17330:
URL: https://github.com/apache/pinot/pull/17330#discussion_r2596252092
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1518,4 +1578,97 @@ public void setSubtaskRunningTimes(Map<String, Long>
subtaskRunningTimes) {
_subtaskRunningTimes = subtaskRunningTimes;
}
}
+
+ /**
+ * Response model for the /tasks/summary endpoint
Review Comment:
The `TaskSummaryResponse` class lacks class-level Javadoc. Add documentation
describing the purpose of this response model and what scenarios it's used for,
especially since it's part of a public API.
```suggestion
* Response model for the /tasks/summary endpoint.
*
* <p>This class encapsulates summary information about tasks currently
managed by the Pinot cluster.
* It is returned by the public API endpoint {@code /tasks/summary} and
provides an overview of
* running and waiting tasks, the number of in-progress task types, and a
breakdown of task types.
*
* <p>Fields:
* <ul>
* <li>{@code totalRunningTasks}: The total number of tasks currently in
the RUNNING state.</li>
* <li>{@code totalWaitingTasks}: The total number of tasks currently in
the WAITING state.</li>
* <li>{@code totalInProgressTaskTypes}: The number of distinct task
types that are currently in progress.</li>
* <li>{@code taskTypeBreakdown}: A list providing detailed breakdowns
for each task type.</li>
* </ul>
*
* <p>This model is intended for use by clients and monitoring tools to
understand the current
* state of task execution in the cluster.
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1518,4 +1578,97 @@ public void setSubtaskRunningTimes(Map<String, Long>
subtaskRunningTimes) {
_subtaskRunningTimes = subtaskRunningTimes;
}
}
+
+ /**
+ * Response model for the /tasks/summary endpoint
+ */
+ @JsonPropertyOrder({"totalRunningTasks", "totalWaitingTasks",
"totalInProgressTaskTypes", "taskTypeBreakdown"})
+ public static class TaskSummaryResponse {
+ private int _totalRunningTasks;
+ private int _totalWaitingTasks;
+ private int _totalInProgressTaskTypes;
+ private List<TaskTypeBreakdown> _taskTypeBreakdown;
+
+ public TaskSummaryResponse() {
+ _totalRunningTasks = 0;
+ _totalWaitingTasks = 0;
+ _totalInProgressTaskTypes = 0;
+ _taskTypeBreakdown = new ArrayList<>();
+ }
+
+ public int getTotalRunningTasks() {
+ return _totalRunningTasks;
+ }
+
+ public void setTotalRunningTasks(int totalRunningTasks) {
+ _totalRunningTasks = totalRunningTasks;
+ }
+
+ public int getTotalWaitingTasks() {
+ return _totalWaitingTasks;
+ }
+
+ public void setTotalWaitingTasks(int totalWaitingTasks) {
+ _totalWaitingTasks = totalWaitingTasks;
+ }
+
+ public int getTotalInProgressTaskTypes() {
+ return _totalInProgressTaskTypes;
+ }
+
+ public void setTotalInProgressTaskTypes(int totalInProgressTaskTypes) {
+ _totalInProgressTaskTypes = totalInProgressTaskTypes;
+ }
+
+ public List<TaskTypeBreakdown> getTaskTypeBreakdown() {
+ return _taskTypeBreakdown;
+ }
+
+ public void setTaskTypeBreakdown(List<TaskTypeBreakdown>
taskTypeBreakdown) {
+ _taskTypeBreakdown = taskTypeBreakdown;
+ }
+ }
+
+ /**
+ * Breakdown of task counts by task type
Review Comment:
The `TaskTypeBreakdown` class lacks class-level Javadoc. Add documentation
describing what this class represents and its fields to improve API clarity.
```suggestion
* Represents a breakdown of task counts by task type for the
/tasks/summary API response.
*
* <p>Fields:
* <ul>
* <li><b>taskType</b>: The type of the task (e.g., segment generation,
validation).</li>
* <li><b>runningCount</b>: The number of tasks of this type currently
running.</li>
* <li><b>waitingCount</b>: The number of tasks of this type currently
waiting to be scheduled.</li>
* </ul>
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -958,6 +958,66 @@ private boolean hasTasksForTable(String taskName, String
tableNameWithType) {
}
}
+ /**
+ * Get a summary of all tasks across all task types.
+ * This consolidates the existing logic from /tasks/tasktypes and
/tasks/{taskType}/taskcounts
+ * into a single endpoint.
+ *
+ * @return TaskSummaryResponse containing aggregated task counts and
breakdown by task type
+ */
+ public synchronized TaskSummaryResponse getTasksSummary() {
Review Comment:
The method is synchronized, which could become a performance bottleneck
under high load since it blocks all concurrent calls to this method. Consider
using a read-write lock or removing synchronization if thread safety is not
required for reading task types and counts.
```suggestion
public TaskSummaryResponse getTasksSummary() {
```
--
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]
