Skip to content
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

Java Rest Client swallows exceptions #39910

Closed
BBK-PiJ-2012-88 opened this issue Mar 11, 2019 · 10 comments
Closed

Java Rest Client swallows exceptions #39910

BBK-PiJ-2012-88 opened this issue Mar 11, 2019 · 10 comments
Labels
:Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch

Comments

@BBK-PiJ-2012-88
Copy link

Describe the feature:

Elasticsearch version (bin/elasticsearch --version): 5.* and 6.*

Plugins installed: []

JVM version (java -version):

OS version (uname -a if on a Unix-like system):

Description of the problem including expected versus actual behavior:

Actual Behaviour

Make a request to an elastic cluster using the java rest client that fails with a transient exception. If the initial request to the cluster takes longer to fail than the max retry timeout millis the request fails with an IOException and the cause of the initial failure is not added as a suppressed exception.

Expected Behaviour:

Make a request to an elastic cluster using the java rest client that fails with a transient exception. If the initial request to the cluster takes longer to fail than the max retry timeout millis the initial reason for the failure should be added to the IOException as a suppressed exception (this is the behaviour if the client attempts 1+ retry attempts).

Steps to reproduce:

This issue is hard to reproduce in a deterministic fashion. However, we encounter the issue fairly frequently with max retry timeout millis set to 1ms.

Provide logs (if relevant):

@javanna
Copy link
Member

javanna commented Mar 11, 2019

I believe this has been fixed with #25576 . Note that we also removed support for maxRetryTimeout in 7.0 with #38085 . WIth earlier versions, I'd recommend against using such a low maxRetryTimeout given the issues that can cause.

@javanna javanna added the :Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch label Mar 11, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@BBK-PiJ-2012-88
Copy link
Author

Hi @javanna, thanks for getting back to me so quickly. That does look like it fixes the issue. We have to support elastic 5 and 6 for our clients. Would it be okay if I raise a pull request to back port the fix for 5?

I agree about the timeout in principle. We set it so low because the retry logic in the client isn't stack safe and blew our stack in prod. I guess I should probably raise an issue on that front...

@javanna
Copy link
Member

javanna commented Mar 11, 2019

Please do raise an issue!

Are you using the low-level REST client or the high-level one?

@BBK-PiJ-2012-88
Copy link
Author

Okay will do.

We're using 'org.elasticsearch.client.RestClient' via the Elastic4s library.

@javanna
Copy link
Member

javanna commented Mar 11, 2019

Ok that sounds like you are using the low-level REST client, which means that you can happily use the 6.6 client version (which contains the linked fix) against a 5.6 Elasticsearch cluster.

@BBK-PiJ-2012-88
Copy link
Author

I'm not sure this will be possible with Elastic4s (standard Scala elastic client library). I'm pretty sure trying to bring in elastic 6 dependencies in conjunction with any Elastic4s 5.* versions will cause a lot of class path problems. #39920

@javanna
Copy link
Member

javanna commented Mar 11, 2019

Ok sounds like Elastic4s depends on the whole Elasticsearch despite it uses low-level REST client to communicate with it. In that case what I suggested will indeed cause problems. The 5.6 branch is not under active development though, hence I doubt that we will back-port that small fix to it at this point.

@BBK-PiJ-2012-88
Copy link
Author

Okay, fair enough

@javanna
Copy link
Member

javanna commented Mar 11, 2019

Closing, see above

@javanna javanna closed this as completed Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch
Projects
None yet
Development

No branches or pull requests

3 participants