Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feature/retry logic in es2plus client #964

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

la3lma
Copy link
Member

@la3lma la3lma commented Nov 26, 2019

Feature to fix bug/misfeature: Some times the ES2+ responds slowly, and this patch gives the client a little more time to get it right before signalling a failure.

Also some refactoring/additional documentation (based on hints from Intellij which checks for such things before committing to VCS).

@la3lma la3lma requested review from kmmatwork and mpeterss November 26, 2019 12:43
@mpeterss
Copy link
Collaborator

What is the timeout for requests set to in the HttpClient?

@la3lma
Copy link
Member Author

la3lma commented Nov 26, 2019

Timeout is set to ten seconds:
httpClient:
timeout: 10000ms

Copy link
Member

@vihangpatil vihangpatil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connection timeout issue can be fixed with just a one-line change in the config.
The current value of 10s is set to timeout.
connectionTimeout is not set at all and hence defaults to 500ms.
Please try that in a separate PR before this elaborate solution.

Reference:
https://www.dropwizard.io/en/stable/manual/configuration.html#httpclient

@la3lma
Copy link
Member Author

la3lma commented Nov 28, 2019

Have submitted a new one-line PR with the change suggested by @vihangpatil. If that does the trick, I will rework this PR into a pure refactoring PR (adding documentation etc.), and remove the retry logic.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants