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..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,8 +25,10 @@ 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; import org.jspecify.annotations.NonNull; /** @@ -35,22 +37,30 @@ * *

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

{@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") public class DefaultFileIOFactory implements FileIOFactory { + private final S3AccessConfig s3AccessConfig; + @Inject - public DefaultFileIOFactory() {} + public DefaultFileIOFactory(S3AccessConfig s3AccessConfig) { + this.s3AccessConfig = s3AccessConfig; + } @Override public FileIO loadFileIO( @NonNull StorageAccessConfig storageAccessConfig, @NonNull String ioImplClassName, - @NonNull Map properties) { + @NonNull Map tableProperties) { // Get subcoped creds - properties = 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 @@ -60,6 +70,53 @@ public FileIO loadFileIO( 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. 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, properties); } 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..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 @@ -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,8 @@ public class WasbTranslatingFileIOFactory implements FileIOFactory { private final FileIOFactory defaultFileIOFactory; @Inject - public WasbTranslatingFileIOFactory() { - defaultFileIOFactory = new DefaultFileIOFactory(); + public WasbTranslatingFileIOFactory(S3AccessConfig s3AccessConfig) { + defaultFileIOFactory = new DefaultFileIOFactory(s3AccessConfig); } @Override 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..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,18 +22,25 @@ 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 * 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}. */ +@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 | |----------|---------------|------|-------------|