http/tcp checks: fix long timeout behavior to default to user-configured value #6094
Conversation
|
|
|
Forgot to update docs! New commit coming.. |
There was a problem hiding this comment.
This looks good @adams-sarah!
However, we should wait to merge for now until we figure out the upcoming release schedule. This will be a significant change for anyone who had a long configured timeout that was getting clipped.
We also need to think about how much we should reword the docs regarding the "interval" flag. We mention in several places that requests are made every interval, but a realization from #5835 was that the we wait for interval time after each check returns (or times out).
The time between checks starting is interval + max(timeout, check duration) rather than just interval:
https://github.com/hashicorp/consul/blob/master/agent/checks/check.go#L99-L101
|
The checks docs should likely look more like this:
|
|
Changed base branch to |
|
@freddygv PTAL @ docs whenever you get a chance! |
| the HTTP response code: any `2xx` code is considered passing, a `429 Too Many | ||
| Requests` is a warning, and anything else is a failure. This type of check | ||
| * HTTP + Interval - These checks make an HTTP `GET` request to the specified URL, | ||
| waiting the specified Interval amount of time (eg. 30 seconds) between requests. |
There was a problem hiding this comment.
I think this reads a little easier if the eg isn't in the middle of the sentence:
| waiting the specified Interval amount of time (eg. 30 seconds) between requests. | |
| waiting between requests for the specified interval time (eg. 30 seconds). |
Also we probably shouldn't be capitalizing "interval" in the description anymore.
There was a problem hiding this comment.
sounds good! shall I codify interval like timeout is? (interval)? that's sort of what i was expecting in the docs
In lieu of #5835 by @pierresouchay (thanks for your PR!!)
Fixes #5834