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