kfaraz commented on code in PR #20505:
URL: https://github.com/apache/druid/pull/20505#discussion_r4259316551
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java:
##########
@@ -506,34 +518,55 @@ public Response getWorkerConfigHistory(
@Path("/action")
@Produces(MediaType.APPLICATION_JSON)
@ResourceFilters(StateResourceFilter.class)
- public Response doAction(final TaskActionHolder holder)
+ public void performTaskAction(
+ final TaskActionHolder holder,
+ @Context HttpServletRequest request
+ )
{
- return asLeaderWith(
- taskMaster.getTaskActionClient(holder.getTask()),
- new Function<>()
+ final AsyncRequest asyncRequest = new AsyncRequest(request);
+ final Optional<TaskActionClient> taskActionClient =
taskMaster.getTaskActionClient(holder.getTask());
+ if (!taskActionClient.isPresent()) {
+ // Encourage client to try again soon, when we'll likely have a redirect
set up
+ asyncRequest.complete(Status.SERVICE_UNAVAILABLE.getStatusCode(), null);
+ return;
+ }
+
+ final ListenableFuture<?> future =
taskActionClient.get().submitAsync(holder.getAction());
+ asyncRequest.context.addListener(
+ ServletResourceUtils.createAsyncTimeoutListener(_ -> {
+ asyncRequest.complete(HttpResponseStatus.GATEWAY_TIMEOUT.code(),
null);
+ future.cancel(true);
+ })
+ );
+
+ // Use a default timeout of 15 minutes
+ asyncRequest.context.setTimeout(15 * 60_000);
+
+ Futures.addCallback(
+ future,
+ new FutureCallback<Object>()
{
@Override
- public Response apply(TaskActionClient taskActionClient)
+ public void onSuccess(Object result)
{
- final Map<String, Object> retMap;
+ // Use null-safe map since some task actions may return null result
+ final Map<String, Object> payload = new HashMap<>();
+ payload.put("result", result);
- // It would be great to verify that this worker is actually
supposed to be running the task before
- // actually doing the action. Some ideas for how that could be
done would be using some sort of attempt_id
- // or token that gets passed around.
+ asyncRequest.complete(Status.OK.getStatusCode(), payload);
+ }
- try {
- final Object ret = taskActionClient.submit(holder.getAction());
- retMap = new HashMap<>();
- retMap.put("result", ret);
- }
- catch (Exception e) {
- log.warn(e, "Failed to perform task action");
- return Response.serverError().entity(ImmutableMap.of("error",
e.getMessage())).build();
+ @Override
+ public void onFailure(Throwable t)
+ {
+ if (t instanceof DruidException druidException) {
+ asyncRequest.complete(druidException.getStatusCode(),
druidException.toErrorResponse());
+ } else {
+
asyncRequest.complete(Status.INTERNAL_SERVER_ERROR.getStatusCode(), null);
Review Comment:
Updated.
--
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]
