klsince commented on code in PR #9540:
URL: https://github.com/apache/pinot/pull/9540#discussion_r988432057


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -434,7 +434,7 @@ public synchronized Map<String, TaskPartitionState> 
getSubtaskStates(String task
    * @param taskName Task name
    * @return List of child task configs
    */
-  public synchronized List<PinotTaskConfig> getTaskConfigs(String taskName) {
+  public synchronized List<PinotTaskConfig> getSubtaskConfigs(String taskName) 
{

Review Comment:
   I assume methods in this class are defined in terms of the abstraction of 
`Minion task` (not `Helix job`, which we use to model a minion task). A minion 
task can have multiple subtasks, and as said in this method comment, it is to 
`Get the child task configs for the given task name`. So I renamed it. 
   
   If keeping the name of this method, I'd call the new method getJobConfig(), 
but we don't expose the `job` concept in the minion framework in most of the 
implementations to my best knowledge. I do see places task/subtask are used 
exchangeably and I had to understand the context to figure out which is which.



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