[GitHub] [druid] LakshSingla commented on a diff in pull request #13062: Use worker number instead of task id in MSQ for communication to/from workers.

[GitBox <gi...@apache.org>]

LakshSingla commented on code in PR #13062:
URL: https://github.com/apache/druid/pull/13062#discussion_r990926670


##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/WorkerClient.java:
##########
@@ -72,7 +72,7 @@ ListenableFuture<Void> postResultPartitionBoundaries(
    * kind of unrecoverable exception).
    */
   ListenableFuture<Boolean> fetchChannelData(
-      String workerTaskId,
+      int workerNumber,

Review Comment:
   I think `WorkerClient` should be refactored to include the worker number to save callers the complexity of resolving the ID to number. In the current indexer's implementation, this is an implementation detail that allows the logic to be wrapped up in the implementation class. 
   The behavior based on fault tolerance would change in the controller task only (as the worker tasks won't be communicating with each other in fault-tolerant mode). In the controller task, irrespective of whether the fault tolerance is enabled, this would resolve to whatever is present in the `MSQWorkerTaskLauncher`, which would ultimately be responsible for determining where the call to a specific worker number should resolve.



-- 
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-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org