abhishekrb19 opened a new pull request, #16618:
URL: https://github.com/apache/druid/pull/16618

   `BrokerClientTest` was added in [PR 
#14322](https://github.com/apache/druid/pull/14322). When running this test 
from the IDE (outside of Maven), `testError()` reliably fails. However, it 
passes in CI due to the Maven setting 
`<surefire.rerunFailingTestsCount>3</surefire.rerunFailingTestsCount>` in the 
[pom.xml](https://github.com/apache/druid/blob/master/pom.xml#L141). This 
setting allows Maven to retry the test up to three times, and the test 
ultimately passes because of the retry logic in the code, but maven treats it 
as "flaky".
   
   ### Changes
   Looking into the error handling in `BrokerClient`, it was found that the 
retry logic doesn't effectively handle 5xx `DruidException` categories since 
the HTTP error codes are converted into `DruidException`s. So the retry logic 
didn't fully account transient errors and this patch fixes that. We could 
perhaps change the semantics of the method to just use the HTTP response, or 
write a service client that uses the built-in mechanisms, but it'll be a larger 
change.
   
   Also, I removed the code that converts all non-200 error codes into 
retryable 5xx DruidException categories. For example, 4xx error codes shouldn't 
be tried. If there are additional 500 error codes we need to retry, we can add 
them explicitly similar to the ones that are currently handled.
   
   This PR has:
   
   - [x] been self-reviewed.
   - [x] added unit tests or modified existing tests to cover new code paths, 
ensuring the threshold for [code 
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
 is met.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to