New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for reactive Elasticsearch healthcheck #21042
Add support for reactive Elasticsearch healthcheck #21042
Conversation
3202327
to
2bd5b1a
Compare
2bd5b1a
to
21af08b
Compare
any ideas why CI failed? Running locally succeeds 😶
|
@aleksanderlech Those failures look unrelated to your changes. We've had some issues with diskspace on the CI box, so it might be that. |
Hello, any feedback here? :) |
Thanks for your contribution! I agree that we should align here with what we're doing with MongoDB (reactive vs. non-reactive). This changes the name of the contributor beans, which is a breaking change. We should consider that in a minor release (at least), but we're already quite late in the 2.3 cycle with RC1 already being released. Also, it bothers me that the health response for the reactive case is very different from the one with the base REST client. Your proposal shows a global "UP"/"DOWN" status and specific "ONLINE"/"OFFLINE" node details. The existing indicator queries the @christophstrobl and @mp911de might disagree with that point, since the client interface only exposes a One last bit: it seems that we've let a typo sneak in (and your contribution aligns there, so not your fault!) - it should be |
There's currently no method to issue a request to client.execute(webClient -> webClient.get().uri("/_cluster/health/").exchange()).flatMap(clientResponse -> clientResponse.bodyToMono(…)) to query the cluster state. It would make sense to provide such a method on |
@mp911de you mean to to add a healthcheck method to |
I think so. Let’s continue the cluster API discussion in a DATAES ticket. |
@bclozel thanks for the feedback, I did not expect it to be merged straight away and actually had the same concerns regarding the cluster state and response model but wanted to get some feedback from the team first. I would be more very happy if I can implement it following your suggestions. |
Prior to this commit, configuring a reactive Elasticsearch client would auto-configure an Actuator Health check using a synchronous client, with the default configuration properties (so tarting localhost:9200). This would lead to false reports of unhealthy Elasticsearch clusters when using reactive clients. This commit reproduces the logic for MongoDB repositories: if a reactive variant is available, it is selected for the health check infrastructure. See gh-21042
Merged with 86d8366 |
Hello,
I am working a lot with spring-data-elasticsearch using reactive repositories. I noticed that even the newest version still supports only healthchecks for ReactiveElasticsearchClient. More over the default healthcheck still wants to run even if all of the clients are reactive - it uses the default one created by autoconfiguration that fails.
I checked how it was done for Mongo that supports reactive indicators as well as basing on the existing ElasticSearch health indicator I implemented a simple proposal. The generated output would be as follows:
This is my first contribution to the project so please forgive me any mistakes, I tried my best to follow the guidelines. Any comments appreciated. Thank you.