Add a bounded Cluster.health(Duration) overload - #26885
Open
patrickmann wants to merge 3 commits into
Open
Conversation
The existing health() cannot be bounded by configuration alone. Its connect (10s) and socket (60s) timeouts apply per host, and the REST client works through every configured node inside a single call, so its worst case is a multiple of both. A caller that must return within a budget of its own -- a health check with an evaluation deadline -- has no way to get one today. Add an overload that issues the request asynchronously and cancels it at the deadline, which bounds wall-clock and stops the client continuing through its remaining hosts. OS2 and ES7 use PlainActionFuture with the *Async client methods, the shape cancellableMsearch already uses in both; OS3 routes through the client's existing executeWithClientTimeout. Each also pulls the request's own cluster-manager timeout down to the same bound, since it defaults to 30s and a cluster with no elected manager would otherwise sit there server-side. A timeout is reported the way an unreachable cluster already is, as an empty Optional: to a caller on a deadline, "did not answer in time" and "did not answer" are the same fact. OS3's bounded variant additionally catches the runtime OpenSearchException an error response produces, which its IOException- only handler lets escape. Purely additive. The five existing health() call sites are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CompletableFuture.get(timeout) clears the interrupt flag when it throws InterruptedException, and the blanket catch (Throwable) wrapped and rethrew it without restoring the flag -- so an interrupt delivered while a bounded call (e.g. the new clusterHealth(Duration)) was waiting was silently lost, defeating cooperative cancellation on the pooled worker. Restore it, mirroring the ES7/OS2 bounded health adapters. Also correct the OS3 bounded-health test comment: the mocked transport delivers the stubbed 500 as an IOException, so the test pins only that an error response folds into empty, not the runtime OpenSearchException path (which the mock cannot produce) nor the deadline firing (which needs a delaying transport). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves the conflict in ClusterAdapterOS.java: master #26836 added countOfClusterManagerEligibleNodes() at the same insertion point where this branch added the bounded clusterHealth(Duration). The two methods are orthogonal; kept both. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
patrickmann
marked this pull request as ready for review
August 5, 2026 07:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
/nocl internal API addition, no user-facing change
Supports Graylog2/graylog-plugin-enterprise#15101 (via Graylog2/graylog-plugin-enterprise#15103).
Description
Cluster.health()cannot be bounded by configuration. Its connect timeout (default 10s) and socket timeout (default 60s) are applied per host, and the REST client works through every configured node inside a single call, so the worst case is a multiple of both. A caller that must return within a deadline of its own has no way to get one today.This PR adds
Cluster.health(java.time.Duration)/ClusterAdapter.health(Duration)overload that issues the request asynchronously and cancels it at the deadline.Per backend:
PlainActionFuturewith the*Asyncclient methods, the same shapecancellableMsearchalready uses in both client classes.executeWithClientTimeout, which already implements this design. It simply was not wired intoclusterHealth().A timeout is reported the way an unreachable cluster already is, as an empty
Optional. To a caller on a deadline, "did not answer in time" and "did not answer" are the same fact. OS3's bounded variant additionally catches the runtimeOpenSearchExceptionan error response produces, which itsIOException-only handler currently lets escape.The change is purely additive. The five existing
health()call sites are untouched and keep the un-timed behaviour.Future utility for other reporters
The immediate consumer is
search_cluster.server.state, but the exposure it fixes is shared by every cluster health reporter that reads the search backend. The health framework gives each reporter a 5s wall-clock budget and can only abandon a result, not kill the worker. Its SPI therefore requires each reporter to bound its own backend call. MongoDB-backed reporters already satisfy this using the driver'swithTimeout, which bounds the whole operation including retries. The search client has no equivalent, so until now search-backed reporters had no way to comply.How Tested
Automated: unit tests
Manual: See the linked enterprise PR for description of manual test coverage
Types of changes
Checklist: