zhtaoxiang commented on code in PR #10132:
URL: https://github.com/apache/pinot/pull/10132#discussion_r1073162110


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -451,6 +455,48 @@ public String getSubtaskProgress(@Context HttpHeaders 
httpHeaders,
     }
   }
 
+  @GET
+  @Path("/tasks/subtask/state/{subTaskState}/progress")

Review Comment:
   Thanks for the suggestions!
   
   I agree with you that the existing pattern is to have `taskType` or 
`taskName` path parameter and we are introducing another pattern here.
   
   Two other choices I have considered
   1. As you mentioned, a single API `GET /tasks/subtask/progress` with all 
different filters (task name, subtask name, subtask state, minion worker ids) 
as query params. 
   
   The problems are 
   (1) not all filter combinations are valid, e.g., without knowing which 
tasks/subtasks are running on which minion workers, it's hard to provide a 
valid combination of (task name/subtask name, minion worker ids)
   (2) it's too generic, so it may be hard for users to understand how to use 
it correctly. (i.e., what to expect with different combinations.)
   (3) it's too generic, so it may also be hard for us to get it right (e.g., 
should we also task progress from task queue if a task is not assigned to a 
minion worker?)
   
   Personally, I prefer specific APIs to serve specific purpose. This API is 
doing one thing: get task process from minion workers.
   
   2. Use the API path `GET /tasks/subtask/progress` with `sub task state and 
minion worker id filers`. 
   
   The name is misleading. The problem is that users may think that this API is 
a super set of the exiting `GET /tasks/subtask/{taskName}/progress`.
   
   Personally, I prefer to go with option 2 if we can come up with a better API 
path.
   
   What do you think?



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