abhishekrb19 opened a new pull request, #19950:
URL: https://github.com/apache/druid/pull/19950
### Description
`HttpLoadQueuePeon` fetches a historical's segment-loading capabilities
synchronously in its constructor, via `GET
/druid-internal/v1/segments/loadCapabilities`.
On any response other than 200 or 404 (e.g. a 503 from an overloaded or
briefly-unavailable historical), the constructor threw an `RE`.
Peon construction happens inside
`LoadQueueTaskMaster.resetPeonsForNewServers()`,
which is the first line of `PrepareBalancerAndLoadQueues` — the first duty
in the
`HistoricalManagementDuties` group. `CoordinatorDutyGroup.run()` has no
per-duty
exception isolation, so a throw there aborted the **entire** group:
`RunRules`,
`BalanceSegments`, `UnloadUnusedSegments`, etc. never ran. The top-level
`DruidCoordinator.DutiesRunnable.run()` catch logged
`"Caught exception, ignoring so that schedule keeps going."` and the
coordinator
kept ticking — but performed no segment management on **any** server.
The blast radius is cluster-wide from a single sick server: segment
loading and
balancing stop, handoffs stall, and ingestion tasks back up waiting for
handoff.
Because `computeIfAbsent` does not cache on exception, the coordinator
re-attempts
the bad server every run and fails again the same way.
Observed in production as slow handoffs and ingestion tasks taking 2x+
longer,
correlated with the alert above whose stack trace pointed at
`HttpLoadQueuePeon` → `resetPeonsForNewServers` →
`PrepareBalancerAndLoadQueues.run`.
### Fix
Make the capabilities fetch degrade gracefully instead of throwing. On a
non-OK
response or any error, alert and fall back to default
`SegmentLoadingCapabilities`
(derived from the configured batch size) — mirroring the behavior the 404
branch
already had. The peon is still created, the server is still managed with
conservative defaults, and the rest of the duty group proceeds. Operators
still
get one alert per affected server.
### Changes
- `HttpLoadQueuePeon.fetchSegmentLoadingCapabilities()`: the non-OK-status
branch
and the catch-all now alert and return default capabilities instead of
throwing.
Extracted a `getDefaultLoadingCapabilities()` helper shared by all three
fallback
paths (404, non-OK, error).
### Tests
- `HttpLoadQueuePeonTest`: constructing a peon against a server that
returns 503 on
`loadCapabilities` now succeeds and falls back to default capabilities.
- `DruidCoordinatorTest#testUnhealthyHistoricalDoesNotAbortDutyGroup`:
end-to-end
test with a real `DruidCoordinator` + `LoadQueueTaskMaster` where one of
three
historicals 503s; asserts the `HistoricalManagementDuties` group still
completes
a run and the two healthy historicals are still managed.
<!-- Replace XXXX with the id of the issue fixed in this PR. Remove this
section if there is no corresponding issue. Don't reference the issue in the
title of this pull-request. -->
<!-- If you are a committer, follow the PR action item checklist for
committers:
https://github.com/apache/druid/blob/master/dev/committer-instructions.md#pr-and-issue-action-item-checklist-for-committers.
-->
This PR has:
- [x] been self-reviewed.
- [x] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [x] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
- [x] been tested in a test Druid cluster.
--
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]
