BewareMyPower opened a new pull request, #17410:
URL: https://github.com/apache/pulsar/pull/17410

   ### Motivation
   
   Currently the operation timeout only works for requests other than
   lookup, like SEND and FLOW. However, the lookup requests, which are sent
   by `LookupService`, should also apply the operation timeout, see
   
   
https://github.com/apache/pulsar/blob/7075a5ce0d4a70f52625ac8c3d0c48894442b72a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java#L1019-L1025
   
   In addition, no attempts would be retried if lookup failed even due to a
   retryable reason. For example, if some of all configured brokers were
   down, the C++ client would fail immediately.
   
   Therefore, this PR intends to retry lookup for some certain cases:
   - The connection cannot be established, except connection timeout, which
     is controlled by the connection timeout.
   - A `ServiceNotReady` error is received, except it's caused by
     `PulsarServerException`, e.g. the listener name is wrong.
   
   Then, apply the operation timeout to avoid infinite retries.
   
   ### Modifications
   
   - Add a `ResultRetryable` error code, which should only be used
     internally. Complete the futures with this error in the cases said
     previously.
   - Add a `RetryableLookupService` implementation to support retries based
     on the backoff policy. Replace the directly usages of `LookupService`
     implementations with this class in `ClientImpl`.
   
   The following tests are added:
   - `ClientTest.testMultiBrokerUrl`: verify when multiple brokers are
     configured, even if one of them is not available, the creation of
     producer or consumer could still succeed.
   - `LookupService.testRetry`: verify all lookup methods could be retried.
   - `LookupService.testTimeout`: verify all lookup methods could be
     completed with `ResultTimeout` if no brokers are available.
   
   ### TODO
   
   In future, we should add lookup timeout instead of operation timeout for
   lookup requests and separate lookup connection pool, see PIP-91.
   
   ### Documentation
   
   Check the box below or label this PR directly.
   
   Need to update docs? 
   
   - [ ] `doc-required` 
   (Your PR needs to update docs and you will update later)
     
   - [x] `doc-not-needed` 
   (Please explain why)
     
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)


-- 
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]

Reply via email to