FrankChen021 commented on code in PR #20505:
URL: https://github.com/apache/druid/pull/20505#discussion_r4229985284
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java:
##########
@@ -506,34 +515,57 @@ public Response getWorkerConfigHistory(
@Path("/action")
@Produces(MediaType.APPLICATION_JSON)
@ResourceFilters(StateResourceFilter.class)
- public Response doAction(final TaskActionHolder holder)
+ public void doAction(
+ final TaskActionHolder holder,
+ @Nullable @PathParam("timeout") final Long timeoutMillis,
Review Comment:
## Follow-up assessment
Agreed, `@QueryParam("timeout")` is the right binding. That fixes the
annotation typo, but the timeout concern also needs the caller/default
addressed: `RemoteTaskActionClient` posts to `/druid/indexer/v1/action` without
a timeout query parameter, so changing the annotation alone still leaves worker
requests capped at five minutes. A prior task group may legitimately remain
pending under the supervisor's default 30-minute `completionTimeout`; the
listener cancels the action instead of returning a retryable
`SegmentPublishResult`.
**Suggestion:** Have worker requests use a deadline that covers the
configured supervisor completion wait (or make the server derive it), and
return an explicit response that the publish caller can safely retry when that
deadline expires. The existing P1 remains actionable at this head; no
additional finding is raised by this follow-up.
Reviewed 7 repository source files for this thread: 3 of the PR's 18 changed
files and 4 supporting files. This was a focused static follow-up, not a full
PR re-review.
---
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request
another review from me to trigger a new automated review.
<!-- mergelens:review -->
--
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]
