aiborodin opened a new pull request, #18515: URL: https://github.com/apache/iceberg/pull/18515
`BaseMetastoreTableOperations` and `BaseViewOperations` retry reading the metadata file during a refresh for up to a hard-coded 10 minutes. When the object store is reachable but unhealthy, for example when reads time out, a single `loadTable` blocks for about 10 minutes before it fails. In a REST catalog server backed by `JdbcCatalog`, this holds a request thread for the whole time, while clients usually give up much earlier. This is similar to #3379, which made the number of metadata refresh retries configurable for `HiveTableOperations` (`iceberg.hive.metadata-refresh-max-retries`). Here the total retry time is configurable instead, because it bounds how long a refresh can block regardless of how long each read attempt takes. Changes: - Add the `metadata-refresh.timeout-ms` property to `CatalogProperties`. The default is 10 minutes, so behavior doesn't change. - Add a protected `metadataRefreshTimeoutMs()` hook to `BaseMetastoreTableOperations` and `BaseViewOperations`, and use it as the total retry time in `refreshFromMetadataLocation`. - `JdbcTableOperations` and `JdbcViewOperations` read the property from the catalog properties and reject negative values. - Document the property for the JDBC catalog. To keep this PR small it only wires up JDBC. Other catalogs can support the property by overriding the hook, as suggested for `BaseMetastoreTableOperations` in #10002. I'm happy to extend it to other catalogs in this PR or in follow-ups. `Tasks` only checks the time limit after a failed attempt, starting from the second attempt. So a refresh always makes at least two attempts, and it can exceed the timeout by one read attempt. The property's Javadoc notes this. Points for reviewers: - The property name `metadata-refresh.timeout-ms` (alternatives: a `jdbc.` prefixed property, or also exposing the retry count). - A protected hook instead of another `refreshFromMetadataLocation` overload, so that existing overloads and subclasses that call them pick up the timeout without changes. Testing: new tests in `TestJdbcCatalog` and `TestJdbcViewCatalog` check that a refresh with a short timeout stops after two failed reads instead of retrying 20 times, and that negative values are rejected. I ran the JDBC test suites, Spotless, checkstyle and revapi for `iceberg-core` locally. I wrote this change with the help of an AI coding assistant (Claude Code), and I reviewed the design and the code and ran the tests myself. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
