ege-st commented on code in PR #13537:
URL: https://github.com/apache/pinot/pull/13537#discussion_r1670541447


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -317,8 +317,10 @@ public synchronized String submitTask(String 
parentTaskName, List<PinotTaskConfi
   public synchronized Set<String> getTasks(String taskType) {
     String helixJobQueueName = getHelixJobQueueName(taskType);
     WorkflowConfig workflowConfig = 
_taskDriver.getWorkflowConfig(helixJobQueueName);
-    Preconditions.checkArgument(workflowConfig != null, "Task queue: %s for 
task type: %s does not exist",
-        helixJobQueueName, taskType);
+    if (workflowConfig == null) {
+      LOGGER.error("Task queue: {} for task type: {} does not exist", 
helixJobQueueName, taskType);

Review Comment:
   Minor question: should this be an `ERROR` level log?  Would this condition 
indicate that the user gave bad information (an invalid task type) or that 
Helix is in a bad state (would it be a bad state if a task does not have an 
associated queue)?
   
   I think if the issue is that the user gave an invalid value for `taskType` 
then the log level should be `INFO`. If the `taskType` is valid but the queue 
does not exist then that could be an `ERROR`.



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