wombatu-kun opened a new pull request, #16437: URL: https://github.com/apache/iceberg/pull/16437
Part of #7528. ## Motivation Retry backoff in `Tasks.runTaskWithRetry` was a single hardcoded exponential-plus-jitter formula with no way to substitute another algorithm. ## What this PR does Introduces a pluggable SPI `org.apache.iceberg.util.BackoffStrategy` selected via a single property `retry.strategy-impl`, loaded reflectively via the standard `DynConstructors` no-arg constructor + `initialize(Map)` pattern that `catalog-impl`, `io-impl`, and `lock-impl` already use. The new `ExponentialBackoffStrategy` reproduces the previous inline formula and jitter exactly, and is the default whenever no strategy is configured — so existing callers see byte-for-byte identical wait times. Every production `.exponentialBackoff(...)` site that has access to a properties map now routes through the SPI and is selectable from its local properties: commits, REST catalog handlers, metadata refresh, status checks, in-memory and DynamoDB lock managers, the Hive Metastore lock (via Hadoop `Configuration`), REST scan polling, and Spark cleanup via `FileIO.properties()`. OAuth2 token refresh has no usable properties map at its call site, so it intentionally stays on the default exponential path. A follow-up PR will add an AIMD strategy on top of this SPI without further core changes. ## Compatibility Behavior is byte-for-byte unchanged unless `retry.strategy-impl` is set. `Tasks.Builder.backoffStrategy(null)` is a no-op, which is what makes `.backoffStrategy(BackoffStrategies.from(props))` work as a one-liner at every call site — `from(...)` returns `null` when the property is absent. The interface and supporting classes are purely additive; no public API removals. ## Tests `TestBackoffStrategies` covers the loader: happy path with `initialize` invocation, null/empty/missing-key returns, unknown class name, missing no-arg constructor, non-strategy class, and `initialize` exception propagation. `TestTasks` covers Tasks integration: custom strategy overrides the default, `null` keeps the default, the returned milliseconds actually drive `Thread.sleep` (timing-verified), and a single strategy instance is shared across parallel worker threads. `TestBackoffStrategyCommit` exercises end-to-end commit retry through the SPI across all four format versions. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
