Allow turn off of doReqAsync adaptive timeout #1152
Conversation
cli/util.go
Outdated
| // Unless: | ||
| // - We are waiting for a specific number of responses (finisher is null) OR | ||
| // - We were told explicitly to wait for the full interval for responses | ||
| if finisher != nil && waitFor != doReqAsyncWaitTimeoutInterval { |
There was a problem hiding this comment.
wouldn't finisher always be nil if waitFor is -1, so just checking if finisher != nil should be sufficient here, if not we should restructure things that the nil check is sufficient, easy to miss multiple places to update if this behaviour changes in future again.
There was a problem hiding this comment.
Actually sorry above is probsbly incorrect. Better behaviour for -1 would be advancing the timer here by opts.Timeout instead of 300ms. Thats it.
This way you get the behaviour if handling responses while they are coming and then getting a 2s by default idle period at the end to allow for slow ones.
Would overall make things slower when there are bad networking interactions but big improvement to reliability and a way to adjust that using the timeout option.
There was a problem hiding this comment.
The default timeout is 5s, so waits 5 extra seconds from the last response.
I think "stop doing smart things and just wait a fixed amount of time" is a more useful "alternative mode" for this utility function.
There was a problem hiding this comment.
Still convinced you should just write your own request method. They have different goals.
There was a problem hiding this comment.
Please take a look at the latest content of the PR, I rolled back all my changes to this function because it already handles "wait for full timeout" if i just pass a negative number.
So I'm just creating a dummy value to make that a little more explicit.
mprimi
changed the title
mprimi
changed the title
When doReqAsync is invoked without a positive 'waitFor' value, it uses a simple adaptive timeout to avoid waiting the full interval (default 5s). This change creates a constant which can be passed into 'waitFor' to disable adaptive timeout and wait for the full interval for responses.
Disable the adaptive responses timeout and wait for the full interval for answers to ping from servers.
doReqAsyncuses an adaptive timeout to stop listening for responses when the number of responses waited is zero.In this PR:
waitFor=-1to wait for the fulltimeoutfor responsesaudit gatherwait for the full timeout