FrankChen021 commented on code in PR #20313:
URL: https://github.com/apache/druid/pull/20313#discussion_r3979036483
##########
multi-stage-query/src/main/java/org/apache/druid/msq/dart/worker/DartWorkerClientImpl.java:
##########
@@ -140,8 +147,26 @@ public void close()
}
@Override
- public ListenableFuture<?> stopWorker(String workerId)
+ public ListenableFuture<Void> postWorkOrder(final String workerId, final
WorkOrder workOrder)
+ {
+ final ListenableFuture<Void> future = super.postWorkOrder(workerId,
workOrder);
Review Comment:
[P1] Check the stop state before dispatching a work order
`stopWorker` sets `isStopping` before it sends the `/stop` request, and
`ControllerImpl.stop` can run concurrently with the controller thread that is
still issuing work orders. This method nevertheless calls `super.postWorkOrder`
before checking that flag, so a post that starts after cancellation has begun
is still handed to `ServiceClient`. In production, canceling the returned
`ServiceClient` future does not reliably abort a request already handed to
`NettyHttpClient` (the client writes the request without a cancellation
listener), so the late `/workOrder` can reach `DartWorkerResource` after
`/stop`; if the old holder has just been removed, that endpoint calls
`startWorker` and can resurrect a canceled query as a zombie worker. Guard
before dispatch and cover the concurrent/late-delivery case, or otherwise make
the worker-side stop state reject such posts.
--
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]
