emanuilov opened a new pull request, #3605: URL: https://github.com/apache/iceberg-rust/pull/3605
## Which issue does this PR close? - Closes [#3166](https://github.com/apache/iceberg-rust/issues/3166). ## What changes are included in this PR? **Problem.** The opendal S3 operator is always wrapped in `RetryLayer::new()`: 3 retries starting at 1 s, factor 2, no jitter. Nothing exposes it (#3263 made the per-operation timeout configurable, not the retry policy). High-throughput writers hit this. One example is a streaming writer into Amazon S3 Tables: a backfill flushing hundreds of partitions at once, or dozens of writers creating files in new tables in one table bucket. S3 answers `503 SlowDown` while it scales the prefix up, which is expected behaviour, and AWS's guidance is to retry with exponential backoff and jitter. With the fixed policy: - the error reaches the caller after about 7 s, usually before S3 has scaled, so the whole commit cycle fails and is redone, adding to the load that caused the throttling; - without jitter, every upload throttled in the same burst retries at the same moments and is throttled again together. The only workaround today is a custom `StorageFactory` that re-implements the opendal storage to change one layer. **Change.** - Adds `s3.retry.num-retries`, `s3.retry.min-wait-ms` and `s3.retry.max-wait-ms` to `iceberg::io` (next to the other `S3_*` constants), with the names and meaning of Iceberg Java's `S3FileIO`, so one catalog configuration behaves the same from Java and Rust. - The opendal S3 operator maps them onto its `RetryLayer` (`with_max_times`, `with_min_delay`, `with_max_delay`). Setting any of them also enables jitter: Java has no jitter key, and the AWS SDK always jitters. - With none set, the layer stays `RetryLayer::new()`, so existing users see no change. - Only the S3 variant applies them; the other opendal backends keep OpenDAL's default. - The keys live in the core crate so other S3 backends can read the same ones. #3165 (the `object_store` backend) has no retry settings yet; they map naturally onto `object_store`'s `RetryConfig`. Related, not dependent. Example, retrying for up to about 2.5 minutes per request: s3.retry.num-retries = 10 s3.retry.min-wait-ms = 2000 s3.retry.max-wait-ms = 20000 ## Are these changes tested? Unit tests in `iceberg-storage-opendal`: - the unset config builds the same layer as `RetryLayer::new()`; - the three properties (and `num-retries = 0`) reach the layer, with jitter on; - zero, negative, fractional and non-numeric values are rejected, with the key in the error; - a non-S3 variant ignores them. `cargo fmt`, `clippy -D warnings` and the tests of `iceberg` (io) and `iceberg-storage-opendal` pass. Not tested against a real throttled store. ## AI Disclosure Drafted with Claude Code (Sonnet 5.5); I reviewed the change and ran fmt, clippy and the tests of the two crates. Points worth a look: - waits must be positive, while `num-retries` may be 0; - setting only some keys leaves the rest at OpenDAL's defaults (1 s / 60 s), not Java's (2 s / 20 s); - `min-wait-ms` greater than `max-wait-ms` is not rejected (OpenDAL's backoff just caps at the maximum); - the properties are parsed for every variant (the client config is shared) but applied only by S3; - `parse_io_timeout_ms` is renamed `parse_positive_ms`, because it now also validates the waits. -- 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]
