sunshowers
commented
2 years ago Thanks for the report! I agree that backoff would be useful. Here are some design thoughts:
Configuration surface
Something like:
retries = { count = 3, exponential-backoff = "1s" }or
retries = { count = 3, fixed-backoff = "1s" }(and not specifying either just means the current default behavior).
Implementation
This is quite simple. This loop:
needs to be updated to simply await on a timer.
Note that I would strongly prefer that the exponential backoff be randomized to avoid thundering herd problems: not just multiplying the time quantum with 2**retry, but picking the multiplier uniformly randomly between 1 and 2**retry.
Reporter surface
It should be made clear in the reporter somehow that a delay is happening. Right now it just says "1/3 RETRY" for example. Something like maybe "1/3 DELAY" with the amount of time the test is being delayed in the message.
Testing
The main things that need to be tested are:
- Parsing valid and invalid retry configuration.
- Maintaining compatibility with existing retry configuration.
- Integration test in
nextest-runnerwhich makes sure that backoff works and is reported.
Documentation
The documentation on retries should be updated to explain these other modes.
Questions
Do we want to allow users to truncate the backoff? If so, we should let that be specified as truncate-at, where the value specifies the retry number at 2**retry that should be specified.
I don't expect to get to this any time soon, would you like to take a crack at it? I think this might end up taking 3-4 days to do.
tomasol
Please consider adding fixed and exponential backoff when retrying failed tests. This is helpful when dealing with rate limiting APIs.