[improve] [pip] PIP-375 Expose the Admin client configs: readTimeout, requestTimeout, and connectionTimeout #23222
Conversation
…Timeout, connectionTimeout
github-actions
bot
added
PIP
doc
poorbarcode
changed the title
poorbarcode
changed the title
pip/pip-375.md
Outdated
| **broker.conf** | ||
| ```properties | ||
| brokerAdminReadTimeoutInSeconds=120 | ||
| brokerAdminRequestTimeoutInSeconds=120 | ||
| brokerAdminConnectionTimeoutInSeconds=30 | ||
| ``` | ||
|
|
||
| ### CLI | ||
|
|
||
| **client.conf** | ||
| ```properties | ||
| adminReadTimeoutInSeconds=300 | ||
| adminRequestTimeoutInSeconds=300 | ||
| adminConnectionTimeoutInSeconds=30 | ||
| ``` |
There was a problem hiding this 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.
There was a problem hiding this comment.
Restored to the value the same as the current default configuration.
pip/pip-375.md
Outdated
| - `readTimeout` | ||
| - `requestTimeout` | ||
| - `connectionTimeout` |
There was a problem hiding this 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".
There was a problem hiding this comment.
Added description
|
|
||
| # Motivation | ||
|
|
||
| We'd better to expose the following configs to users: |
There was a problem hiding this comment.
There was a problem hiding this comment.
It would be good to update this information in the PIP document.
| # The value is the timeout to build a connection to the server. Value 0 represents infinity. Negative values are not allowed. | ||
| connectionTimeout | ||
|
|
||
| # The value is the timeout to read a response. If the server doesn't respond within the defined timeframe, ProcessingException is thrown with TimeoutException as a cause. |
There was a problem hiding this comment.
Is it possible that the server responds, but the client just doesn't know about this? 🤔
What if the server responds, but the response isn't fully received by the client?
Is "response timeout" different from "read timeout"?
Could we improve the definition to reduce the confusion?
There was a problem hiding this comment.
It's better to have clear descriptions for the settings. You could use GitHub Copilot to revisit the comment based on the feedback to assist in coming up with an improvement.
Motivation
Modifications
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: x