Kurtiscwright commented on code in PR #2455:
URL: https://github.com/apache/iceberg-rust/pull/2455#discussion_r3319895821
##########
crates/storage/opendal/src/lib.rs:
##########
@@ -326,9 +326,17 @@ impl OpenDalStorage {
}
};
- // Transient errors are common for object stores; however there's no
- // harm in retrying temporary failures for other storage backends as
well.
- let operator = operator.layer(RetryLayer::new());
+ // Apply observability/resilience layers. TimeoutLayer must be
+ // inside RetryLayer so each retry attempt is independently
+ // bounded — without a per-attempt timeout, a future parked on a
+ // silently dropped TCP connection never produces an `Err` and
+ // RetryLayer cannot retry, leaving the caller hung indefinitely.
+ // See:
https://opendal.apache.org/docs/rust/opendal/layers/struct.TimeoutLayer.html
+ //
+ // Transient errors are common for object stores; we retry temporary
+ // failures with exponential backoff. The retry behavior also
+ // benefits non-object-store backends.
+ let operator =
operator.layer(TimeoutLayer::new()).layer(RetryLayer::new());
Review Comment:
The original feature request linked to a Table with both min and max
timeouts, how does this get encoded here?
https://iceberg.apache.org/docs/latest/configuration/#table-behavior-properties
--
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]
