This repository has been archived by the owner on Jul 27, 2023. It is now read-only.
Fix blocking query read timeout issue #289
Merged
Conversation
This file contains 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
|
Hi @rickfast , |
Currently, the read timeout is the default value of okhttp i.e. 10 seconds. That is a very long duration when we consider testing. Decreasing it to 2 second should have limited to no impact.
This is useful for test purposes.
|
I like it. Rebase it and I’ll merge it. I’ve had a lot of requests and complaints regarding the mismatch between http timeouts and blocking consul waits |
The first test checks that the cache is notified at startup. The second test checks cache notifications for blocking queries of 1 and 10 seconds. Note that it is successful for 1 sec (<readTime) but failing for 10 sec.
This "interceptor" is required to extract the wait query parameter from the url. Then, it adjusts the OkHttp read timeout accordingly. This feature requires OkHttp 3.9.0 which has been already added to retrofit2, but not yet release.
This was referenced Dec 27, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR aims at resolving the read timeout issue for caches (issues #255 and #202)
Using Interceptor (OkHttp > 3.9.0), the timeout can be adjusted depending on the url.
IMO, this is an elegant solution has not change is required for users.
An alternative would have been to use the @headers annotation in the retrofit api, together with a OkHttp interceptor to check the header and adjust the timeout. However, some queries are used for both cache and standard API call, and so queries would need to be duplicated, ...
What has been done: