umatt1 opened a new pull request, #17786: URL: https://github.com/apache/iceberg/pull/17786
Closes #15587 ## Motivation `GCSFileIO` always builds its `Storage` client with the library's default HTTP transport timeouts, which are effectively unbounded for reads (`java.net.URLConnection` blocks indefinitely by default). There is no way to tune this for degraded network conditions. The AWS module already exposes the equivalent knobs via `HttpClientProperties` (`http-client.*-timeout-ms`); this closes that gap for GCS. A prior PR for this issue (#15626) went stale from a rebase gap rather than any design objection — reviewers explicitly pushed back on the stale-close at the time. This PR is scoped narrower: only the timeout configuration requested in #15587, without the unrelated #15411 changes that PR also carried. ## Changes Two new optional properties, following the existing `GCPProperties` conventions: - `gcs.http.connect-timeout-ms` - `gcs.http.read-timeout-ms` When either is set, `PrefixedStorage` builds an `HttpTransportOptions` and wires it into the `StorageOptions.Builder`, in the same place `serviceHost`/`projectId` are already applied. When unset, behavior is byte-for-byte identical to before (no `setTransportOptions` call is made). Purely additive — no signature or behavior changes for existing users. One behavior worth reviewer attention, documented in the Javadoc: these timeouts apply **per HTTP attempt**, not per overall call. The Storage client retries failed requests with backoff by default (gax default total retry timeout ~50s), so a configured read timeout bounds each attempt, not total wall time. ## Testing - `TestGCPProperties`: properties parse correctly and remain unset by default - `TestPrefixedStorage#httpTimeoutsAreWired` / `#httpTimeoutsNotSetByDefault`: values land on the real `HttpTransportOptions` of the constructed client; defaults untouched when unset - `TestPrefixedStorage#readTimeoutIsActuallyEnforced`: functional test against a local socket that accepts the connection but never responds — the request fails with `Read timed out` / `SocketTimeoutException` in ~1s instead of hanging on the unbounded default. Uses `maxAttempts(1)` to isolate a single attempt from the default retry policy (which is what surfaced the per-attempt semantics above). `./gradlew :iceberg-gcp:check` passes (tests, checkstyle, spotless). ## AI disclosure This PR was developed with AI assistance (Claude Code), per the [AI-assisted contribution guidelines](https://iceberg.apache.org/contribute/#guidelines-for-ai-assisted-contributions). All code and tests were run and verified locally; the per-attempt timeout semantics called out above were discovered through the functional test rather than assumed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WVZAaD5G3sWVKsFi8sYDE9 -- 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]
