[GitHub] [pulsar] merlimat edited a comment on issue #4018: Support multi-host for pulsar-admin

[GitBox <gi...@apache.org>]

merlimat edited a comment on issue #4018: Support multi-host for pulsar-admin
URL: https://github.com/apache/pulsar/pull/4018#issuecomment-492413337
 
 
   > If "fail immediately" means fail if not connect, that's probably the case. but that should be respect to connect timeout anyway. The whole problem was raised is because of the commit in #4235 since it raises the default value from 60s to 5 mins.
   
   That was a symptom. The underlying problem is that without this PR, a connect failure would lead to immediate failure.  After this PR the behavior change because we keep doing the same request in a tight loop until the request timeout expires.
   
   > I am not sure how it is different between multi-host or single host.
   
   My point is that if this PR is adding the support for multi-host, it shouldn't change the existing behavior for single-host requests.
   
   > I agreed that this PR can be improved with changes in async paths, adding backoffs or other improvements you highlighted. 
   
   I don't see the implementation in the async path as an improvement. Currently the multi-host request are broken if you happen to use a method in the admin API which is implemented in async mode.
   
   > but this PR doesn't change the contract that pulsar admin cli draws with users.
   
   It doesn't change the fundamental contract, but it does change the behavior, even for single host.
   
   If we want to add HTTP client retries and stuff, that would be more carefully planned and tested. And it would have to go in a different PR.
   
   
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services