From a5ce9afad04eff8e40f607a4fa480cb5c38fa53c Mon Sep 17 00:00:00 2001 From: prithvi Date: Wed, 8 Jul 2026 13:51:42 +0530 Subject: [PATCH 1/3] Propagate storage HTTP client settings to S3FileIO for table operations --- CHANGELOG.md | 1 + .../catalog/io/DefaultFileIOFactory.java | 64 +++++++++++++++++-- .../service/storage/aws/S3AccessConfig.java | 4 +- 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7abb675ad4f..9f3b8837012 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti ### Deprecations ### Fixes +- `polaris.storage.max-http-connections` (and related read/connect/acquisition/idle timeouts) now take effect for Iceberg S3FileIO clients used in table operations (previously only affected the STS client pool). - Fixed a boundary condition in GCS downscoped credential generation (`GcpCredentialsStorageIntegration`). Locations without a trailing slash could previously grant access to sibling object prefixes via the generated CEL conditions for `resource.name` and list prefixes. Granted paths are now normalized to a directory prefix (with a trailing slash) before the CEL conditions are built, so sibling prefixes can no longer satisfy the `startsWith` checks. - Async task execution (table cleanup, manifest and batch file cleanup) now retries when a handler returns false on transient errors (e.g. IO or delete failures). Previously `false` was swallowed with only a warning log and the task was never retried via the existing retry mechanism. - Fixed `NullPointerException` during `dropEntity` when an entity referenced by a grant had been concurrently removed (or purged). `lookupEntities` can return null entries for dropped entities; these are now skipped safely. diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index cf334b2afd7..5e6006ca9d6 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -27,6 +27,7 @@ import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.io.FileIO; import org.apache.polaris.core.storage.StorageAccessConfig; +import org.apache.polaris.service.storage.aws.S3AccessConfig; import org.jspecify.annotations.NonNull; /** @@ -40,8 +41,16 @@ @Identifier("default") public class DefaultFileIOFactory implements FileIOFactory { + private final S3AccessConfig s3AccessConfig; + + public DefaultFileIOFactory() { + this(null); + } + @Inject - public DefaultFileIOFactory() {} + public DefaultFileIOFactory(S3AccessConfig s3AccessConfig) { + this.s3AccessConfig = s3AccessConfig; + } @Override public FileIO loadFileIO( @@ -50,17 +59,60 @@ public FileIO loadFileIO( @NonNull Map properties) { // Get subcoped creds - properties = new HashMap<>(properties); + Map props = new HashMap<>(properties); // Update the FileIO with the subscoped credentials // Update with properties in case there are table-level overrides the credentials should // always override table-level properties, since storage configuration will be found at // whatever entity defines it - properties.putAll(storageAccessConfig.credentials()); - properties.putAll(storageAccessConfig.extraProperties()); - properties.putAll(storageAccessConfig.internalProperties()); + props.putAll(storageAccessConfig.credentials()); + props.putAll(storageAccessConfig.extraProperties()); + props.putAll(storageAccessConfig.internalProperties()); + + // Apply polaris.storage.* HTTP client settings to Iceberg S3FileIO (and other AWS FileIOs). + // Previously only applied to the STS client pool. + if (s3AccessConfig != null) { + s3AccessConfig + .maxHttpConnections() + .ifPresent(v -> props.put("http-client.apache.max-connections", String.valueOf(v))); + s3AccessConfig + .readTimeout() + .ifPresent( + d -> props.put("http-client.apache.socket-timeout-ms", String.valueOf(d.toMillis()))); + s3AccessConfig + .connectTimeout() + .ifPresent( + d -> + props.put( + "http-client.apache.connection-timeout-ms", String.valueOf(d.toMillis()))); + s3AccessConfig + .connectionAcquisitionTimeout() + .ifPresent( + d -> + props.put( + "http-client.apache.connection-acquisition-timeout-ms", + String.valueOf(d.toMillis()))); + s3AccessConfig + .connectionMaxIdleTime() + .ifPresent( + d -> + props.put( + "http-client.apache.connection-max-idle-time-ms", + String.valueOf(d.toMillis()))); + s3AccessConfig + .connectionTimeToLive() + .ifPresent( + d -> + props.put( + "http-client.apache.connection-time-to-live-ms", + String.valueOf(d.toMillis()))); + s3AccessConfig + .expectContinueEnabled() + .ifPresent( + v -> props.put("http-client.apache.expect-continue-enabled", String.valueOf(v))); + } - return loadFileIOInternal(ioImplClassName, properties); + return loadFileIOInternal(ioImplClassName, props); } @VisibleForTesting diff --git a/runtime/service/src/main/java/org/apache/polaris/service/storage/aws/S3AccessConfig.java b/runtime/service/src/main/java/org/apache/polaris/service/storage/aws/S3AccessConfig.java index 940d03229ae..b46f1e478d3 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/storage/aws/S3AccessConfig.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/storage/aws/S3AccessConfig.java @@ -27,8 +27,8 @@ * Configuration interface containing parameters for clients accessing S3 services from Polaris * servers. * - *

Currently, this configuration does not apply to all of Polaris code, but only to select - * services. + *

Applies to the STS client pool and to Iceberg S3 clients created for table operations via + * {@code S3FileIO}. */ public interface S3AccessConfig { /** Default value for {@link #clientsCacheMaxSize()}. */ From 28f0bc4af0859d9d53f10b4840b34ba921e0e0e6 Mon Sep 17 00:00:00 2001 From: prithvi Date: Thu, 9 Jul 2026 00:54:36 +0530 Subject: [PATCH 2/3] Apply storage HTTP client settings on wasb FileIOFactory path Wire S3AccessConfig into WasbTranslatingFileIOFactory so production non-default factories also propagate polaris.storage.* HTTP settings to S3FileIO. Clarify that the no-arg DefaultFileIOFactory path is for tests and fixtures only. --- .../polaris/service/catalog/io/DefaultFileIOFactory.java | 9 ++++++++- .../service/catalog/io/WasbTranslatingFileIOFactory.java | 7 +++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index 5e6006ca9d6..443208e7ece 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -36,6 +36,11 @@ * *

This class acts as a translation layer between Polaris properties and the properties required * by Iceberg's {@link FileIO}. + * + *

When constructed with {@link S3AccessConfig} (CDI {@code @Identifier("default")} and other + * production factories such as {@code wasb}), {@code polaris.storage.*} HTTP client settings are + * applied to Iceberg AWS FileIOs. The no-arg constructor is for tests and fixtures that do not need + * those settings. */ @RequestScoped @Identifier("default") @@ -43,6 +48,7 @@ public class DefaultFileIOFactory implements FileIOFactory { private final S3AccessConfig s3AccessConfig; + /** For tests and fixtures that do not need {@code polaris.storage.*} HTTP client settings. */ public DefaultFileIOFactory() { this(null); } @@ -70,7 +76,8 @@ public FileIO loadFileIO( props.putAll(storageAccessConfig.internalProperties()); // Apply polaris.storage.* HTTP client settings to Iceberg S3FileIO (and other AWS FileIOs). - // Previously only applied to the STS client pool. + // Previously only applied to the STS client pool. Skipped when constructed without config + // (tests / fixtures); production CDI paths always inject S3AccessConfig. if (s3AccessConfig != null) { s3AccessConfig .maxHttpConnections() diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java index cb11456160e..b8d2bbf1757 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java @@ -24,6 +24,7 @@ import java.util.Map; import org.apache.iceberg.io.FileIO; import org.apache.polaris.core.storage.StorageAccessConfig; +import org.apache.polaris.service.storage.aws.S3AccessConfig; import org.jspecify.annotations.NonNull; /** A {@link FileIOFactory} that translates WASB paths to ABFS ones */ @@ -34,8 +35,10 @@ public class WasbTranslatingFileIOFactory implements FileIOFactory { private final FileIOFactory defaultFileIOFactory; @Inject - public WasbTranslatingFileIOFactory() { - defaultFileIOFactory = new DefaultFileIOFactory(); + public WasbTranslatingFileIOFactory(S3AccessConfig s3AccessConfig) { + // Pass storage HTTP client settings through so S3FileIO used under wasb gets the same + // polaris.storage.* tuning as the CDI "default" FileIOFactory path. + defaultFileIOFactory = new DefaultFileIOFactory(s3AccessConfig); } @Override From 6f0fd02344bdebfbf6e5c68ba20423aaae644dec Mon Sep 17 00:00:00 2001 From: prithvi Date: Sat, 11 Jul 2026 02:28:27 +0530 Subject: [PATCH 3/3] review changes Address review feedback: use S3AccessConfig.empty() via @PolarisImmutable instead of null, rename loadFileIO tableProperties param, and reuse Iceberg HttpClientProperties constants. Keep regenerated polaris.storage config docs in the same commit for CI. --- .../catalog/io/DefaultFileIOFactory.java | 112 +++++++++--------- .../io/WasbTranslatingFileIOFactory.java | 2 - .../service/storage/aws/S3AccessConfig.java | 7 ++ .../service/catalog/io/FileIOFactoryTest.java | 3 +- .../catalog/io/MeasuredFileIOFactory.java | 3 +- .../smallrye-polaris_storage.md | 2 +- 6 files changed, 67 insertions(+), 62 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index 443208e7ece..99459ee854f 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.Map; import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.aws.HttpClientProperties; import org.apache.iceberg.io.FileIO; import org.apache.polaris.core.storage.StorageAccessConfig; import org.apache.polaris.service.storage.aws.S3AccessConfig; @@ -37,10 +38,9 @@ *

This class acts as a translation layer between Polaris properties and the properties required * by Iceberg's {@link FileIO}. * - *

When constructed with {@link S3AccessConfig} (CDI {@code @Identifier("default")} and other - * production factories such as {@code wasb}), {@code polaris.storage.*} HTTP client settings are - * applied to Iceberg AWS FileIOs. The no-arg constructor is for tests and fixtures that do not need - * those settings. + *

{@code polaris.storage.*} HTTP client settings from {@link S3AccessConfig} are applied to + * Iceberg AWS FileIOs. Production CDI paths inject the live config; tests/fixtures can pass {@link + * S3AccessConfig#empty()}. */ @RequestScoped @Identifier("default") @@ -48,11 +48,6 @@ public class DefaultFileIOFactory implements FileIOFactory { private final S3AccessConfig s3AccessConfig; - /** For tests and fixtures that do not need {@code polaris.storage.*} HTTP client settings. */ - public DefaultFileIOFactory() { - this(null); - } - @Inject public DefaultFileIOFactory(S3AccessConfig s3AccessConfig) { this.s3AccessConfig = s3AccessConfig; @@ -62,64 +57,67 @@ public DefaultFileIOFactory(S3AccessConfig s3AccessConfig) { public FileIO loadFileIO( @NonNull StorageAccessConfig storageAccessConfig, @NonNull String ioImplClassName, - @NonNull Map properties) { + @NonNull Map tableProperties) { // Get subcoped creds - Map props = new HashMap<>(properties); + Map properties = new HashMap<>(tableProperties); // Update the FileIO with the subscoped credentials // Update with properties in case there are table-level overrides the credentials should // always override table-level properties, since storage configuration will be found at // whatever entity defines it - props.putAll(storageAccessConfig.credentials()); - props.putAll(storageAccessConfig.extraProperties()); - props.putAll(storageAccessConfig.internalProperties()); + properties.putAll(storageAccessConfig.credentials()); + properties.putAll(storageAccessConfig.extraProperties()); + properties.putAll(storageAccessConfig.internalProperties()); // Apply polaris.storage.* HTTP client settings to Iceberg S3FileIO (and other AWS FileIOs). - // Previously only applied to the STS client pool. Skipped when constructed without config - // (tests / fixtures); production CDI paths always inject S3AccessConfig. - if (s3AccessConfig != null) { - s3AccessConfig - .maxHttpConnections() - .ifPresent(v -> props.put("http-client.apache.max-connections", String.valueOf(v))); - s3AccessConfig - .readTimeout() - .ifPresent( - d -> props.put("http-client.apache.socket-timeout-ms", String.valueOf(d.toMillis()))); - s3AccessConfig - .connectTimeout() - .ifPresent( - d -> - props.put( - "http-client.apache.connection-timeout-ms", String.valueOf(d.toMillis()))); - s3AccessConfig - .connectionAcquisitionTimeout() - .ifPresent( - d -> - props.put( - "http-client.apache.connection-acquisition-timeout-ms", - String.valueOf(d.toMillis()))); - s3AccessConfig - .connectionMaxIdleTime() - .ifPresent( - d -> - props.put( - "http-client.apache.connection-max-idle-time-ms", - String.valueOf(d.toMillis()))); - s3AccessConfig - .connectionTimeToLive() - .ifPresent( - d -> - props.put( - "http-client.apache.connection-time-to-live-ms", - String.valueOf(d.toMillis()))); - s3AccessConfig - .expectContinueEnabled() - .ifPresent( - v -> props.put("http-client.apache.expect-continue-enabled", String.valueOf(v))); - } + // Previously only applied to the STS client pool. Empty config is a no-op (tests/fixtures). + s3AccessConfig + .maxHttpConnections() + .ifPresent( + v -> properties.put(HttpClientProperties.APACHE_MAX_CONNECTIONS, String.valueOf(v))); + s3AccessConfig + .readTimeout() + .ifPresent( + d -> + properties.put( + HttpClientProperties.APACHE_SOCKET_TIMEOUT_MS, String.valueOf(d.toMillis()))); + s3AccessConfig + .connectTimeout() + .ifPresent( + d -> + properties.put( + HttpClientProperties.APACHE_CONNECTION_TIMEOUT_MS, + String.valueOf(d.toMillis()))); + s3AccessConfig + .connectionAcquisitionTimeout() + .ifPresent( + d -> + properties.put( + HttpClientProperties.APACHE_CONNECTION_ACQUISITION_TIMEOUT_MS, + String.valueOf(d.toMillis()))); + s3AccessConfig + .connectionMaxIdleTime() + .ifPresent( + d -> + properties.put( + HttpClientProperties.APACHE_CONNECTION_MAX_IDLE_TIME_MS, + String.valueOf(d.toMillis()))); + s3AccessConfig + .connectionTimeToLive() + .ifPresent( + d -> + properties.put( + HttpClientProperties.APACHE_CONNECTION_TIME_TO_LIVE_MS, + String.valueOf(d.toMillis()))); + s3AccessConfig + .expectContinueEnabled() + .ifPresent( + v -> + properties.put( + HttpClientProperties.APACHE_EXPECT_CONTINUE_ENABLED, String.valueOf(v))); - return loadFileIOInternal(ioImplClassName, props); + return loadFileIOInternal(ioImplClassName, properties); } @VisibleForTesting diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java index b8d2bbf1757..d5f98ee2230 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java @@ -36,8 +36,6 @@ public class WasbTranslatingFileIOFactory implements FileIOFactory { @Inject public WasbTranslatingFileIOFactory(S3AccessConfig s3AccessConfig) { - // Pass storage HTTP client settings through so S3FileIO used under wasb gets the same - // polaris.storage.* tuning as the CDI "default" FileIOFactory path. defaultFileIOFactory = new DefaultFileIOFactory(s3AccessConfig); } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/storage/aws/S3AccessConfig.java b/runtime/service/src/main/java/org/apache/polaris/service/storage/aws/S3AccessConfig.java index b46f1e478d3..935770be80d 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/storage/aws/S3AccessConfig.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/storage/aws/S3AccessConfig.java @@ -22,6 +22,7 @@ import java.time.Duration; import java.util.Optional; import java.util.OptionalInt; +import org.apache.polaris.immutables.PolarisImmutable; /** * Configuration interface containing parameters for clients accessing S3 services from Polaris @@ -30,10 +31,16 @@ *

Applies to the STS client pool and to Iceberg S3 clients created for table operations via * {@code S3FileIO}. */ +@PolarisImmutable public interface S3AccessConfig { /** Default value for {@link #clientsCacheMaxSize()}. */ int DEFAULT_MAX_STS_CLIENT_CACHE_ENTRIES = 50; + /** An empty config with no HTTP client overrides (e.g. tests and fixtures). */ + static S3AccessConfig empty() { + return ImmutableS3AccessConfig.builder().build(); + } + /** Maximum number of entries to keep in the STS clients cache. */ OptionalInt clientsCacheMaxSize(); diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java index a1f122774cf..98fc0646ebd 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -48,6 +48,7 @@ import org.apache.polaris.service.TestServices; import org.apache.polaris.service.catalog.PolarisPassthroughResolutionView; import org.apache.polaris.service.catalog.iceberg.LocalIcebergCatalog; +import org.apache.polaris.service.storage.aws.S3AccessConfig; import org.apache.polaris.service.task.TaskFileIOSupplier; import org.assertj.core.api.Assertions; import org.jspecify.annotations.NonNull; @@ -105,7 +106,7 @@ public void before(TestInfo testInfo) { Supplier fileIOFactorySupplier = () -> Mockito.spy( - new DefaultFileIOFactory() { + new DefaultFileIOFactory(S3AccessConfig.empty()) { @Override FileIO loadFileIOInternal( @NonNull String ioImplClassName, @NonNull Map properties) { diff --git a/runtime/service/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIOFactory.java b/runtime/service/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIOFactory.java index 29c29625ed5..cbdce1d2998 100644 --- a/runtime/service/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIOFactory.java +++ b/runtime/service/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIOFactory.java @@ -27,6 +27,7 @@ import java.util.function.Supplier; import org.apache.iceberg.io.FileIO; import org.apache.polaris.core.storage.StorageAccessConfig; +import org.apache.polaris.service.storage.aws.S3AccessConfig; import org.jspecify.annotations.NonNull; /** @@ -47,7 +48,7 @@ public class MeasuredFileIOFactory implements FileIOFactory { @Inject public MeasuredFileIOFactory() { - defaultFileIOFactory = new DefaultFileIOFactory(); + defaultFileIOFactory = new DefaultFileIOFactory(S3AccessConfig.empty()); } @Override diff --git a/site/content/in-dev/unreleased/configuration/config-sections/smallrye-polaris_storage.md b/site/content/in-dev/unreleased/configuration/config-sections/smallrye-polaris_storage.md index 9e039160234..f7af9089a8b 100644 --- a/site/content/in-dev/unreleased/configuration/config-sections/smallrye-polaris_storage.md +++ b/site/content/in-dev/unreleased/configuration/config-sections/smallrye-polaris_storage.md @@ -25,7 +25,7 @@ build: Configuration interface containing parameters for clients accessing S3 services from Polaris servers. -Currently, this configuration does not apply to all of Polaris code, but only to select services. +Applies to the STS client pool and to Iceberg S3 clients created for table operations via `S3FileIO`. | Property | Default Value | Type | Description | |----------|---------------|------|-------------|