mattp5657 opened a new issue, #4084: URL: https://github.com/apache/iggy/issues/4084
### Description `exponential_backoff`/`jitter` in `core/connectors/sdk/src/retry.rs` have two bugs, both reproduced by every call site that composes them by hand: 1. **Off-by-one doubles the first retry's delay.** `exponential_backoff(base, attempt, max_delay)` expects a 0-based `attempt` (`0` → `base`). Most call sites increment their counter *before* calling it, so the first retry passes `1` and waits `base * 2` instead of `base`, and every subsequent retry is skewed one step ahead too. 2. **Jitter is applied after the cap, so it can overshoot `max_delay`.** `exponential_backoff` clamps to `max_delay` internally, but `jitter(...)` (±20%) wraps that already-capped value with no re-clamp: a retry can sleep up to `max_delay * 1.2`, silently exceeding the configured ceiling. ### Affected area / component Connectors, Rust SDK ### Proposed solution Fix the interface, not every call site: change `exponential_backoff` to accept a 1-based total-attempt count internally (subtract 1 before computing the exponent), so the call every caller already makes naturally (an incremented counter) becomes correct. - **No change needed:** SDK's `HttpRetryMiddleware`/`check_connectivity_with_retry` (fixes `influxdb_sink`/`influxdb_source` too), `meilisearch_sink`, `core/connectors/sources/iggy_source`. - **Drop the now-redundant manual `-1`:** `s3_sink`, `surrealdb_sink`, `doris_sink`, `opensearch_sink`, SDK `source.rs::nack_retry_delay`. - **Separate fix, unaffected by the interface change:** `clickhouse_sink`'s local `jittered_backoff` (own `attempts - 1` fix, 4 call sites). Also add `.min(max_delay)` after `jitter(...)` once, in a shared `retry_backoff(base, attempt, max_delay) -> Duration` helper, and have every call site use it instead of composing `jitter`/`exponential_backoff` by hand. This closes the clamp gap in the same pass since those call sites are already being touched. ### Alternatives considered Fix each call site individually instead of the function. The current 0-based contract is the one nearly every caller gets wrong (SDK's own middleware, `meilisearch_sink`, and `iggy_source` all pass an already-incremented counter). Since the interface itself is being misread by most callers, fixing it at the source removes the problem for good instead of leaving it for the next connector to repeat. ### Contribution - [ ] I'm willing to submit a pull request to implement this feature ### Good first issue - [x] I think this could be a good first issue for a new contributor -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
