docs: clarify waitTimeout Javadoc for WaiterOverrideConfiguration - #7292
Conversation
Add a paragraph to WaiterOverrideConfiguration.Builder#waitTimeout(Duration) that explains the interaction between waitTimeout and maxAttempts. Customers frequently expect waitTimeout alone to extend a waiter's total wait time. Because services provide their own default maxAttempts (with no service default for waitTimeout), setting waitTimeout longer than the service-default budget (maxAttempts x delay-between-polls) causes the maxAttempts limit to terminate the waiter first (see #5838). The new paragraph: - explains that waitTimeout, when set, works alongside maxAttempts (which caps the number of polling attempts and has a service-provided default) - states that the waiter transitions to a failure state as soon as either limit is reached - directs customers wanting to extend the wait to override maxAttempts to increase the number of polling attempts or backoffStrategyV2 to lengthen the delay between polls The wording follows the "transitions to a failure state" phrasing already used in the sibling maxAttempts(Integer) Javadoc in the same file. No source-code / API changes.
| * timeout doesn't have strict guarantees on how quickly a request is aborted when the timeout is breached. The request | ||
| * can timeout early if it is determined that the next retry will breach the max wait time. It's disabled by default. | ||
| * | ||
| * <p>When set, {@code waitTimeout} works alongside {@link #maxAttempts(Integer)}, which caps the number of polling |
There was a problem hiding this comment.
Nits :
-
Can we frame
waitTimeoutas an upper bound rather than a target, and if possible add the concrete Unexpected behavior from waiter if timeout provided #5838 case so it is clear the waiter errors out at the attempt limit, not after the full timeout? -
Can we add the note on
maxAttemptstoo so both setters describe the interaction saying maxAttempts works with waitTimeout and waiters transitions to a failure state as soon as it reaches either limit.
There was a problem hiding this comment.
-
Sure, the upper-bound framing with an example helps, revised.
-
Thought about it initially, and scoped to waitTimeout since it's null by default. The interaction only kicks in when a customer explicitly sets it, and the misinterpretation was directional. But only fair to make it symmetric, added it.
- waitTimeout: reframe as upper bound; add concrete #5838 example (maxAttempts=100 x 6s -> waiter terminates at ~10 min even with waitTimeout=30 min) - maxAttempts: add reciprocal interaction note so both setters describe the two-limit stop-condition behavior No source-code / API changes.
|
This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one. |
Motivation and Context
Issue #5838 reported that setting
WaiterOverrideConfiguration.waitTimeout(Duration.ofMinutes(30))on the ECSTasksRunningwaiter did not produce a 30-minute wait — the waiter terminated at ~10 minutes with"The waiter has exceeded the max retry attempts: 100".This is the as-designed behavior:
waitTimeoutandmaxAttemptsare independent limits, and whichever is reached first transitions the waiter to a failure state. For the ECSTasksRunningservice defaults (delay=6s,maxAttempts=100), the total service-default wait budget is ~10 minutes, so a longerwaitTimeoutcannot extend past that ceiling — themaxAttemptslimit fires first.The confusion stems from the current Javadoc not documenting this interaction. Customers reasonably assume
waitTimeoutis a primary "how long to wait" knob when it is actually an optional cap that operates alongside the service-provided defaultmaxAttempts.Related to #5838.
Modifications
Doc-only edit to
WaiterOverrideConfiguration.Builder#waitTimeout(Duration). Adds a paragraph that:waitTimeout, when set, works alongsidemaxAttempts(which caps the number of polling attempts and has a service-provided default)maxAttemptsorbackoffStrategyV2The new paragraph uses "transitions to a failure state" phrasing already established in the sibling
maxAttempts(Integer)Javadoc in the same file.No API or source-code changes.
Testing
mvn clean install -pl :sdk-core -P quick --am— sdk-core builds cleanly with the change{@link}references verified against the file's existing style anddocs/guidelines/javadoc-guidelines.mdLicense