Also add note about default maxRetries of 3 to maxDuration section of the documentation
stefanrybacki opened this issue · 7 comments
The default maxRetries setting of 3 is mentioned in the retries section, however the duration section https://failsafe.dev/retry/#duration reads as if maxRetries is then determined by the max duration.
So you want to emphasize that maxDuration doesn't supersede maxRetries/Attempts? Would this addition make it clear?
You can add a max duration for an execution, after which retries will stop (if not stopped by other constraints, like
maxRetries/Attempts).
Yes exactly I ran into this today wondering why only 3 attempts were taking place, only rereading the duration section I was confused. However, after reading the javadoc of withMaxDuration it dawned on me.
Regarding your proposed wording I still find it missleading as I never specified maxRetries so I would not assume other constraints as I already have specified a stop constraint via duration. So maybe also add a note to your proposal, something along the lines:
You can add a max duration for an execution, after which retries will stop (if not stopped by other constraints, like maxRetries/Attempts, Note: maxRetries is set to 3 if not overridden).
@jhalterman - Would you want a wording change here, or an earlier emphasis on how maxRetries/Attempts is always considered, or both?
Maybe a wording change that does both...
It might be good to move the "Duration" section of the Retry docs to be right below the "Attempts" section, since they both serve the same purpose of constraining executions, as you point out. For proposed wording on "Duration", which would directly follow "Attempts", maybe:
In addition to max attempts, you can also add a max duration for an execution, after which retries will stop if the max attempts haven't already been reached.
Would those two changes clarify things?
@jhalterman sounds good, I could also image that an example with both maxDuration and maxRetries set to -1 would also make it even clearer
I just pushed this change and also added a note about disabling retries: failsafe-lib/failsafe.dev@4ae3ec3
@jhalterman much better now, thank you for the quick fix