fix(iceberg_fdw): [WRA-27] configurable HTTP timeouts for REST catalog#617
Merged
Conversation
The Iceberg REST catalog client had no timeout, so a stalled connection to the catalog server would hang the query indefinitely, uninterruptible by statement_timeout or pg_cancel_backend, pinning VACUUM/replication slot horizons in the process (WRA-27). Adds request_timeout_ms/connect_timeout_ms server options and injects a timeout-configured reqwest client into RestCatalogBuilder. This requires bumping reqwest to 0.12 workspace-wide (with matching reqwest-middleware/ reqwest-retry/http bumps) since iceberg-catalog-rest pins reqwest 0.12.12.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds configurable HTTP connect/request timeouts to the Iceberg REST catalog client to prevent indefinitely hung FDW queries, and updates the workspace’s HTTP client dependencies to reqwest 0.12-compatible versions.
Changes:
- Add
request_timeout_ms(default 30000) andconnect_timeout_ms(default 10000) server options and apply them via a timeout-configuredreqwest::Clienton the REST catalog path. - Add a regression pg_test to ensure an unreachable REST catalog fails quickly instead of hanging.
- Bump
reqwest/reqwest-middleware/reqwest-retry/httpversions workspace-wide and document the new options.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wrappers/src/fdw/iceberg_fdw/iceberg_fdw.rs | Builds a timeout-configured reqwest client and injects it into the REST catalog builder. |
| wrappers/src/fdw/iceberg_fdw/mod.rs | Adds reqwest error plumbing to the Iceberg FDW error type. |
| wrappers/src/fdw/iceberg_fdw/tests.rs | Adds a regression test to assert REST-catalog unreachability is bounded by timeouts. |
| wrappers/Cargo.toml | Enables reqwest for iceberg_fdw and bumps reqwest/http-related dependency versions. |
| docs/catalog/iceberg.md | Documents the new timeout server options and defaults. |
| Cargo.lock | Locks updated dependency graph from the reqwest/http version bumps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Read request_timeout_ms/connect_timeout_ms from the vault-decrypted props map instead of raw server.options, so vault_-prefixed keys aren't silently ignored. - Use an RFC 5737 TEST-NET address (192.0.2.1) instead of an RFC 1918 address in the timeout test, since the latter can be routable in some CI/VPC environments.
burmecia
changed the title
imor
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RestCatalogBuilder::default()) had no request/connect timeout, so a stalled connection to the catalog server hangs the query indefinitely — uninterruptible bystatement_timeout/pg_cancel_backend/pg_terminate_backend, pinningOldestXminand logical replication slot horizons while blocked.request_timeout_ms(default 30000) andconnect_timeout_ms(default 10000), and injects a timeout-configuredreqwest::ClientintoRestCatalogBuilder::with_client(...)on the REST catalog path only (S3Tables catalog is unaffected).iceberg-catalog-rest0.8.0 pinsreqwest = "0.12.12", incompatible with the workspace's existingreqwest 0.11.20. Rather than adding a separately-renamed dependency, this bumpsreqwest/reqwest-middleware/reqwest-retry/httpworkspace-wide to versions compatible with 0.12. Surveyed every FDW gating these crates (airtable, auth0, cognito, firebase, logflare, stripe, s3_fdw) — all recompile with no code changes needed.