[REST] Rest client should catch NoHttpResponse Exception and retry#4334
Closed
zwangsheng wants to merge 2 commits intoapache:masterfrom
zwangsheng:rest_client/retry_with_no_http_response
Closed
[REST] Rest client should catch NoHttpResponse Exception and retry#4334zwangsheng wants to merge 2 commits intoapache:masterfrom zwangsheng:rest_client/retry_with_no_http_response
NoHttpResponse Exception and retry#4334zwangsheng wants to merge 2 commits intoapache:masterfrom
zwangsheng:rest_client/retry_with_no_http_response
Conversation
Contributor
Author
zwangsheng
changed the title
NoHttpResponse Exception and retry
bowenliang123
approved these changes
Feb 15, 2023
ulysses-you
approved these changes
Feb 15, 2023
Codecov Report
@@ Coverage Diff @@
## master #4334 +/- ##
============================================
- Coverage 53.58% 53.56% -0.02%
Complexity 13 13
============================================
Files 562 562
Lines 30704 30704
Branches 4142 4142
============================================
- Hits 16452 16446 -6
- Misses 12707 12714 +7
+ Partials 1545 1544 -1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
pan3793
pushed a commit
that referenced
this pull request
Feb 15, 2023
…ion` and retry ### _Why are the changes needed?_ [NoHttpResponseException](https://hc.apache.org/httpclient-legacy/exception-handling.html) > In some circumstances, usually when under heavy load, the web server may be able to receive requests but unable to process them. A lack of sufficient resources like worker threads is a good example. This may cause the server to drop the connection to the client without giving any response. HttpClient throws NoHttpResponseException when it encounters such a condition. In most cases it is safe to retry a method that failed with NoHttpResponseException. In case of Kyuubi Server is overloaded and does not have enough resources to allocate threads to handle the corresponding request, the request will be dropped and no response will be returned, Kyuubi Rest Client should catch this exception and retry. ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request Closes #4334 from zwangsheng/rest_client/retry_with_no_http_response. Closes #4334 d526e4a [zwangsheng] Remove Unrelated Code 708f249 [zwangsheng] [REST] Retry should catch NoHttpResponse Exception and retry Authored-by: zwangsheng <2213335496@qq.com> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit a17ccdf) Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793
pushed a commit
that referenced
this pull request
Feb 15, 2023
…ion` and retry ### _Why are the changes needed?_ [NoHttpResponseException](https://hc.apache.org/httpclient-legacy/exception-handling.html) > In some circumstances, usually when under heavy load, the web server may be able to receive requests but unable to process them. A lack of sufficient resources like worker threads is a good example. This may cause the server to drop the connection to the client without giving any response. HttpClient throws NoHttpResponseException when it encounters such a condition. In most cases it is safe to retry a method that failed with NoHttpResponseException. In case of Kyuubi Server is overloaded and does not have enough resources to allocate threads to handle the corresponding request, the request will be dropped and no response will be returned, Kyuubi Rest Client should catch this exception and retry. ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request Closes #4334 from zwangsheng/rest_client/retry_with_no_http_response. Closes #4334 d526e4a [zwangsheng] Remove Unrelated Code 708f249 [zwangsheng] [REST] Retry should catch NoHttpResponse Exception and retry Authored-by: zwangsheng <2213335496@qq.com> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit a17ccdf) Signed-off-by: Cheng Pan <chengpan@apache.org>
Member
|
Thanks, merged to master/1.7/1.6 |
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
5 participants
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.
Why are the changes needed?
NoHttpResponseException
In case of Kyuubi Server is overloaded and does not have enough resources to allocate threads to handle the corresponding request, the request will be dropped and no response will be returned, Kyuubi Rest Client should catch this exception and retry.
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request