Skip to content

Conversation

@vovacf201
Copy link
Collaborator

@vovacf201 vovacf201 commented Nov 10, 2025

Available Settings for Retry/Timeout

Available Settings for Retry/Timeout

io.retry.max-attempts - Maximum retry attempts (default: 3)
io.retry.min-delay-ms - Minimum delay between retries (default: 1000)
io.retry.max-delay-ms - Maximum delay between retries (default: 60000)
io.retry.factor - Exponential backoff factor (default: 2.0)
io.timeout-ms - Operation timeout in milliseconds (optional, no default)
@Li0k Li0k requested a review from Copilot December 3, 2025 05:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds configurable timeout and retry settings for OpenDAL operations in Iceberg's FileIO implementation. The changes allow users to fine-tune timeout duration and retry behavior (max attempts, min/max delays) through configuration properties instead of relying solely on OpenDAL's defaults.

Key changes:

  • Added four new configuration constants for timeout and retry settings
  • Modified create_operator to accept configuration and apply timeout/retry layers based on provided settings
  • Updated all FileIO methods to pass configuration when creating operators

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.

File Description
crates/iceberg/src/io/file_io.rs Defines new configuration constants (IO_TIMEOUT_SECONDS, IO_MAX_RETRIES, IO_RETRY_MIN_DELAY_MS, IO_RETRY_MAX_DELAY_MS) and updates FileIO methods to pass configuration to operator creation
crates/iceberg/src/io/storage.rs Renames and updates create_operator to create_operator_with_config, adds logic to parse and apply timeout/retry configuration from provided HashMap

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Li0k
Copy link

Li0k commented Dec 4, 2025

Hi, @vovacf201. This PR is generally LGTM.

The current code implementation is a bit redundant. Could you refer to the copilot implementation and revise it first?

@vovacf201
Copy link
Collaborator Author

Hi, @vovacf201. This PR is generally LGTM.

The current code implementation is a bit redundant. Could you refer to the copilot implementation and revise it first?

should be fixed now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants