cryptoe commented on code in PR #13062:
URL: https://github.com/apache/druid/pull/13062#discussion_r995354141
##########
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:
The worker-client needs to be a simple client. Given a taskId, do the
required operation.
I think that would be a more manageable contract for the users of the client
as it is deterministic.
When the msqWokerTaskLauncher changes something in the background, we do not
want an operation to succeed on a worker in the wrong state.
Users will always have to look up the taskID and get the worker state which
is on the worker number.
As we have async api's it can be very well possible that we end up getting
success from a task that according to the controller state is in retrying. ie
the worker is in retrying.
In such a case we simply have to ignore the state change which becomes hard
if we have only the worker id and not the taskId. We won't know if the taskID
is to be ignored or not as the worker state in the controller is on worker id.
Another reason which tipped me over towards the taskID approach is that
worker-side client interface changes are not required in the worker JVM.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
