lhotari commented on code in PR #23222: URL: https://github.com/apache/pulsar/pull/23222#discussion_r1732810117
########## pip/pip-375.md: ########## @@ -0,0 +1,41 @@ +# PIP-375: Expose the Admin client configs: readTimeout, requestTimeout, connectionTimeout + +# Background knowledge + +- `pulsar-admin sinks create` will upload files to brokers, but if the file is large and the network is slow, it may cost a long time. +- The default value of `requestTimeout` of Admin API is `60s`, after the fix https://github.com/apache/pulsar/pull/23128, it was changed to `300s`. + +# Motivation + +We'd better to expose the following configs to users: +- `readTimeout` +- `requestTimeout` +- `connectionTimeout` + +# Goals + +Expose the Admin client configs: `readTimeout`, `requestTimeout` and `connectionTimeout` to users. + +# Detailed Design + +### Configuration + +**broker.conf** +```properties +brokerAdminReadTimeoutInSeconds=120 +brokerAdminRequestTimeoutInSeconds=120 +brokerAdminConnectionTimeoutInSeconds=30 +``` + +### CLI + +**client.conf** +```properties +adminReadTimeoutInSeconds=300 +adminRequestTimeoutInSeconds=300 +adminConnectionTimeoutInSeconds=30 +``` Review Comment: Are these values intended to be good sample values? In other sources, I think that read timeout is typically set to a rather short timeout. ########## pip/pip-375.md: ########## @@ -0,0 +1,41 @@ +# PIP-375: Expose the Admin client configs: readTimeout, requestTimeout, connectionTimeout + +# Background knowledge + +- `pulsar-admin sinks create` will upload files to brokers, but if the file is large and the network is slow, it may cost a long time. +- The default value of `requestTimeout` of Admin API is `60s`, after the fix https://github.com/apache/pulsar/pull/23128, it was changed to `300s`. + +# Motivation + +We'd better to expose the following configs to users: +- `readTimeout` +- `requestTimeout` +- `connectionTimeout` Review Comment: Please add a short definition of these configuration parameters. One reason is that there has been misconceptions about readTimeout and requestTimeout as described in #23128. There's also a possibility for confusion about "connection timeout". One has to think whether it means "connect timeout" or "connection idle timeout", or something else. Based on the javadoc it means a "connect timeout". -- 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]
