From bd7c8c9ef3e15dd29c61353a40d466301211417b Mon Sep 17 00:00:00 2001 From: aiborodin Date: Mon, 19 Oct 2026 16:39:57 +1100 Subject: [PATCH 1/2] Core: Make metadata refresh retries configurable in JDBC catalog During a storage outage, loading a table or view retries the metadata read 20 times for up to 10 minutes, stalling clients. Add the jdbc.metadata-refresh-max-retries catalog property (default 20) to bound it. --- .../org/apache/iceberg/jdbc/JdbcCatalog.java | 13 ++++- .../iceberg/jdbc/JdbcTableOperations.java | 7 ++- .../org/apache/iceberg/jdbc/JdbcUtil.java | 18 +++++++ .../iceberg/jdbc/JdbcViewOperations.java | 20 +++++++- .../apache/iceberg/jdbc/TestJdbcCatalog.java | 40 ++++++++++++++++ .../iceberg/jdbc/TestJdbcViewCatalog.java | 47 +++++++++++++++++-- docs/docs/jdbc.md | 1 + 7 files changed, 138 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java b/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java index 0d98b53e721e..3191caa51e2a 100644 --- a/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java +++ b/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java @@ -93,6 +93,7 @@ public class JdbcCatalog extends BaseMetastoreViewCatalog private JdbcClientPool connections; private Map catalogProperties; private boolean uniqueTableLocation; + private int metadataRefreshMaxRetries; private final Function, FileIO> ioBuilder; private final Function, JdbcClientPool> clientPoolBuilder; private boolean initializeCatalogTables; @@ -130,6 +131,7 @@ public void initialize(String name, Map properties) { properties, CatalogProperties.UNIQUE_TABLE_LOCATION, CatalogProperties.UNIQUE_TABLE_LOCATION_DEFAULT); + this.metadataRefreshMaxRetries = JdbcUtil.metadataRefreshMaxRetries(properties); if (name != null) { this.catalogName = name; @@ -284,7 +286,13 @@ private static boolean executeV1CatalogUpdate(Connection conn) throws SQLExcepti @Override protected TableOperations newTableOps(TableIdentifier tableIdentifier) { return new JdbcTableOperations( - connections, io, catalogName, tableIdentifier, catalogProperties, schemaVersion); + connections, + io, + catalogName, + tableIdentifier, + catalogProperties, + schemaVersion, + metadataRefreshMaxRetries); } @Override @@ -292,7 +300,8 @@ protected ViewOperations newViewOps(TableIdentifier viewIdentifier) { if (schemaVersion != JdbcUtil.SchemaVersion.V1) { throw new UnsupportedOperationException(VIEW_WARNING_LOG_MESSAGE); } - return new JdbcViewOperations(connections, io, catalogName, viewIdentifier, catalogProperties); + return new JdbcViewOperations( + connections, io, catalogName, viewIdentifier, catalogProperties, metadataRefreshMaxRetries); } @Override diff --git a/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java b/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java index 079faf1c5504..53dd7a8313ec 100644 --- a/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java +++ b/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java @@ -49,6 +49,7 @@ class JdbcTableOperations extends BaseMetastoreTableOperations { private final JdbcClientPool connections; private final Map catalogProperties; private final JdbcUtil.SchemaVersion schemaVersion; + private final int metadataRefreshMaxRetries; protected JdbcTableOperations( JdbcClientPool dbConnPool, @@ -56,13 +57,15 @@ protected JdbcTableOperations( String catalogName, TableIdentifier tableIdentifier, Map catalogProperties, - JdbcUtil.SchemaVersion schemaVersion) { + JdbcUtil.SchemaVersion schemaVersion, + int metadataRefreshMaxRetries) { this.catalogName = catalogName; this.tableIdentifier = tableIdentifier; this.fileIO = fileIO; this.connections = dbConnPool; this.catalogProperties = catalogProperties; this.schemaVersion = schemaVersion; + this.metadataRefreshMaxRetries = metadataRefreshMaxRetries; } @Override @@ -96,7 +99,7 @@ public void doRefresh() { newMetadataLocation != null, "Invalid table %s: metadata location is null", tableIdentifier); - refreshFromMetadataLocation(newMetadataLocation); + refreshFromMetadataLocation(newMetadataLocation, metadataRefreshMaxRetries); } @Override diff --git a/core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java b/core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java index 528eccdf16c9..4eaa837059c0 100644 --- a/core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java +++ b/core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java @@ -44,6 +44,10 @@ final class JdbcUtil { // property to control if catalog tables are created during initialization static final String INIT_CATALOG_TABLES_PROPERTY = JdbcCatalog.PROPERTY_PREFIX + "init-catalog-tables"; + // property to control how many times reading table or view metadata is retried during a refresh + static final String METADATA_REFRESH_MAX_RETRIES_PROPERTY = + JdbcCatalog.PROPERTY_PREFIX + "metadata-refresh-max-retries"; + static final int METADATA_REFRESH_MAX_RETRIES_DEFAULT = 20; static final String RETRYABLE_STATUS_CODES = "retryable_status_codes"; private static final String POSTGRES_UNIQUE_VIOLATION_SQLSTATE = "23505"; @@ -528,6 +532,20 @@ static Properties filterAndRemovePrefix(Map properties, String p return result; } + static int metadataRefreshMaxRetries(Map properties) { + int maxRetries = + PropertyUtil.propertyAsInt( + properties, + METADATA_REFRESH_MAX_RETRIES_PROPERTY, + METADATA_REFRESH_MAX_RETRIES_DEFAULT); + Preconditions.checkArgument( + maxRetries >= 0, + "Invalid %s: %s (must be >= 0)", + METADATA_REFRESH_MAX_RETRIES_PROPERTY, + maxRetries); + return maxRetries; + } + static boolean isConstraintViolation(SQLException ex) { return ex instanceof SQLIntegrityConstraintViolationException || POSTGRES_UNIQUE_VIOLATION_SQLSTATE.equals(ex.getSQLState()) diff --git a/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java b/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java index 646e5b860625..6a7e5c77c17f 100644 --- a/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java +++ b/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java @@ -49,6 +49,7 @@ public class JdbcViewOperations extends BaseViewOperations { private final FileIO fileIO; private final JdbcClientPool connections; private final Map catalogProperties; + private final int metadataRefreshMaxRetries; protected JdbcViewOperations( JdbcClientPool dbConnPool, @@ -56,11 +57,28 @@ protected JdbcViewOperations( String catalogName, TableIdentifier viewIdentifier, Map catalogProperties) { + this( + dbConnPool, + fileIO, + catalogName, + viewIdentifier, + catalogProperties, + JdbcUtil.metadataRefreshMaxRetries(catalogProperties)); + } + + JdbcViewOperations( + JdbcClientPool dbConnPool, + FileIO fileIO, + String catalogName, + TableIdentifier viewIdentifier, + Map catalogProperties, + int metadataRefreshMaxRetries) { this.catalogName = catalogName; this.viewIdentifier = viewIdentifier; this.fileIO = fileIO; this.connections = dbConnPool; this.catalogProperties = catalogProperties; + this.metadataRefreshMaxRetries = metadataRefreshMaxRetries; } @Override @@ -90,7 +108,7 @@ protected void doRefresh() { String newMetadataLocation = view.get(JdbcTableOperations.METADATA_LOCATION_PROP); Preconditions.checkState( newMetadataLocation != null, "Invalid view %s: metadata location is null", viewIdentifier); - refreshFromMetadataLocation(newMetadataLocation); + refreshFromMetadataLocation(newMetadataLocation, null, metadataRefreshMaxRetries); } @Override diff --git a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java index 225e6da20a9e..3039dfead37c 100644 --- a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java +++ b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java @@ -24,9 +24,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.times; import java.io.File; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; @@ -54,6 +57,7 @@ import org.apache.iceberg.SortOrder; import org.apache.iceberg.Table; import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.TableOperations; import org.apache.iceberg.TableProperties; import org.apache.iceberg.Transaction; @@ -65,6 +69,7 @@ import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.iceberg.hadoop.Util; +import org.apache.iceberg.io.FileIO; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -1067,6 +1072,41 @@ public void testRemoveProperties() { assertThat(catalog.namespaceExists(testNamespace)).isTrue(); } + @Test + void metadataRefreshMaxRetries() { + try (JdbcCatalog jdbcCatalog = + initCatalog( + "test_jdbc_catalog_refresh_retries", + ImmutableMap.of(JdbcUtil.METADATA_REFRESH_MAX_RETRIES_PROPERTY, "1"))) { + TableIdentifier tableIdent = TableIdentifier.of("db", "tbl"); + jdbcCatalog.buildTable(tableIdent, SCHEMA).create(); + + try (MockedStatic mockedStatic = + Mockito.mockStatic(TableMetadataParser.class)) { + mockedStatic + .when(() -> TableMetadataParser.read(any(FileIO.class), anyString())) + .thenThrow(new UncheckedIOException(new IOException("Read timed out"))); + + assertThatThrownBy(() -> jdbcCatalog.loadTable(tableIdent)) + .isInstanceOf(UncheckedIOException.class) + .hasMessageContaining("Read timed out"); + mockedStatic.verify( + () -> TableMetadataParser.read(any(FileIO.class), anyString()), times(2)); + } + } + } + + @Test + void invalidMetadataRefreshMaxRetries() { + assertThatThrownBy( + () -> + initCatalog( + "test_jdbc_catalog_invalid_refresh_retries", + ImmutableMap.of(JdbcUtil.METADATA_REFRESH_MAX_RETRIES_PROPERTY, "-1"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid jdbc.metadata-refresh-max-retries: -1 (must be >= 0)"); + } + @Test public void testConversions() { Namespace ns = Namespace.of("db", "db2", "ns2"); diff --git a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java index b9f51dc5d93b..d08792fb921e 100644 --- a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java +++ b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java @@ -21,8 +21,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.times; import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; import java.sql.SQLException; import java.util.Map; import java.util.UUID; @@ -33,10 +37,13 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.io.FileIO; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.view.BaseView; import org.apache.iceberg.view.ViewCatalogTests; import org.apache.iceberg.view.ViewMetadata; +import org.apache.iceberg.view.ViewMetadataParser; import org.apache.iceberg.view.ViewOperations; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -52,6 +59,10 @@ public class TestJdbcViewCatalog extends ViewCatalogTests { @BeforeEach public void before() { + catalog = initCatalog(ImmutableMap.of()); + } + + private JdbcCatalog initCatalog(Map additionalProperties) { Map properties = Maps.newHashMap(); properties.put( CatalogProperties.URI, @@ -65,10 +76,12 @@ public void before() { properties.put(CatalogProperties.VIEW_DEFAULT_PREFIX + "key3", "catalog-default-key3"); properties.put(CatalogProperties.VIEW_OVERRIDE_PREFIX + "key3", "catalog-override-key3"); properties.put(CatalogProperties.VIEW_OVERRIDE_PREFIX + "key4", "catalog-override-key4"); + properties.putAll(additionalProperties); - catalog = new JdbcCatalog(); - catalog.setConf(new Configuration()); - catalog.initialize("testCatalog", properties); + JdbcCatalog jdbcCatalog = new JdbcCatalog(); + jdbcCatalog.setConf(new Configuration()); + jdbcCatalog.initialize("testCatalog", properties); + return jdbcCatalog; } @Override @@ -91,6 +104,34 @@ protected boolean supportsEmptyNamespace() { return true; } + @Test + void metadataRefreshMaxRetries() { + try (JdbcCatalog jdbcCatalog = + initCatalog(ImmutableMap.of(JdbcUtil.METADATA_REFRESH_MAX_RETRIES_PROPERTY, "0"))) { + TableIdentifier identifier = TableIdentifier.of("namespace1", "view"); + jdbcCatalog.createNamespace(identifier.namespace()); + jdbcCatalog + .buildView(identifier) + .withQuery("spark", "select * from tbl") + .withSchema(SCHEMA) + .withDefaultNamespace(identifier.namespace()) + .create(); + + try (MockedStatic mockedStatic = + Mockito.mockStatic(ViewMetadataParser.class)) { + mockedStatic + .when(() -> ViewMetadataParser.read(any(FileIO.class), anyString())) + .thenThrow(new UncheckedIOException(new IOException("Read timed out"))); + + assertThatThrownBy(() -> jdbcCatalog.loadView(identifier)) + .isInstanceOf(UncheckedIOException.class) + .hasMessageContaining("Read timed out"); + mockedStatic.verify( + () -> ViewMetadataParser.read(any(FileIO.class), anyString()), times(1)); + } + } + } + @Test public void testCommitExceptionWithoutMessage() { TableIdentifier identifier = TableIdentifier.of("namespace1", "view"); diff --git a/docs/docs/jdbc.md b/docs/docs/jdbc.md index b1717096a768..8281cf7af298 100644 --- a/docs/docs/jdbc.md +++ b/docs/docs/jdbc.md @@ -35,6 +35,7 @@ the JDBC catalog allows arbitrary configurations through: | -------------------- | --------------------------------- | ------------------------------------------------------ | | uri | | the JDBC connection string | | jdbc. | | any key value pairs to configure the JDBC connection | +| jdbc.metadata-refresh-max-retries | 20 | number of times to retry reading table or view metadata during a refresh | ### Examples From 9a71b4b0280fbb733baefd20058fbbb44125bc5b Mon Sep 17 00:00:00 2001 From: aiborodin Date: Tue, 20 Oct 2026 16:44:09 +1100 Subject: [PATCH 2/2] Use the existing test naming in TestJdbcCatalog and TestJdbcViewCatalog --- .../test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java | 4 ++-- .../java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java index 3039dfead37c..fbc79574aafc 100644 --- a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java +++ b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java @@ -1073,7 +1073,7 @@ public void testRemoveProperties() { } @Test - void metadataRefreshMaxRetries() { + public void testMetadataRefreshMaxRetries() { try (JdbcCatalog jdbcCatalog = initCatalog( "test_jdbc_catalog_refresh_retries", @@ -1097,7 +1097,7 @@ void metadataRefreshMaxRetries() { } @Test - void invalidMetadataRefreshMaxRetries() { + public void testInvalidMetadataRefreshMaxRetries() { assertThatThrownBy( () -> initCatalog( diff --git a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java index d08792fb921e..4d8665a5513e 100644 --- a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java +++ b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java @@ -105,7 +105,7 @@ protected boolean supportsEmptyNamespace() { } @Test - void metadataRefreshMaxRetries() { + public void testMetadataRefreshMaxRetries() { try (JdbcCatalog jdbcCatalog = initCatalog(ImmutableMap.of(JdbcUtil.METADATA_REFRESH_MAX_RETRIES_PROPERTY, "0"))) { TableIdentifier identifier = TableIdentifier.of("namespace1", "view");