MartijnVisser commented on code in PR #27026:
URL: https://github.com/apache/flink/pull/27026#discussion_r4087304686
##########
flink-filesystems/flink-s3-fs-hadoop/pom.xml:
##########
@@ -84,12 +84,43 @@ under the License.
<scope>test</scope>
</dependency>
- <!-- S3 base -->
+ <!--
+ This plugin builds on Hadoop's s3a connector
(hadoop-aws), which migrated
+ to AWS SDK v2 (`software.amazon.awssdk:*`) in the
Hadoop 3.4.x line. The
+ v2 modules we actually bundle are declared explicitly
further down.
+
+ The shared `flink-s3-fs-base` parent still pulls AWS
SDK v1
+ (`com.amazonaws:*`) transitively because the sibling
`flink-s3-fs-presto`
+ plugin remains on v1, since we are using AWS SDK V2
with 3.4.x, we don't
+ need AWS SDK V1 (`com.amazonaws:*`)
+ -->
<dependency>
<groupId>org.apache.flink</groupId>
<artifactId>flink-s3-fs-base</artifactId>
<version>${project.version}</version>
<optional>${flink.markBundledAsOptional}</optional>
+ <exclusions>
+ <exclusion>
+ <groupId>com.amazonaws</groupId>
+
<artifactId>aws-java-sdk-core</artifactId>
Review Comment:
Without SDK v1 here, unmapped v1 provider names fail:
`DefaultAWSCredentialsProviderChain` works on master, throws
`ClassNotFoundException` here. Pre-setting
`s3.aws.credentials.provider.mapping` for common ones works (tried it); custom
providers then need docs and a suggested release note.
##########
docs/content/docs/deployment/filesystems/s3.md:
##########
@@ -228,6 +228,33 @@ cp ./opt/flink-s3-fs-hadoop-{{< version >}}.jar
./plugins/s3-fs-hadoop/
The [common configuration](#common-configuration) options apply. In addition,
[Hadoop's s3a configuration
keys](https://hadoop.apache.org/docs/stable/hadoop-aws/tools/hadoop-aws/index.html#S3A)
are supported. Hadoop configuration keys are automatically translated — for
example, `fs.s3a.connection.maximum` becomes `s3.connection.maximum`.
+**Input stream type:** hadoop-aws 3.4.3 changes its default input stream to
the [S3 Analytics
Accelerator](https://hadoop.apache.org/docs/stable/hadoop-aws/tools/hadoop-aws/stream_architecture.html)
(`fs.s3a.input.stream.type: analytics`), which alters read patterns,
per-stream memory usage, and the volume of S3 GET requests. To keep upgrades
behavior-preserving, Flink pins the classic input stream by default. To opt in
to the analytics stream, set `s3.input.stream.type: analytics`; setting the
legacy `s3.prefetch.enabled: true` toggle is also respected and selects
Hadoop's prefetching stream.
+
+#### Changed defaults from the Hadoop 3.4 upgrade
+
+The Hadoop S3 FileSystem passes Hadoop's s3a defaults through unchanged.
Hadoop 3.4 retuned several of them as part of its migration to the AWS SDK v2;
Flink deliberately keeps the new values (the input stream type above is the
only pinned exception), so that the client behaves the way the Hadoop community
tunes and tests it. The table below lists the changed defaults and why upstream
changed them. Any key can be restored through the `s3.` passthrough shown
underneath.
+
+| Key | Hadoop 3.3 default | Hadoop 3.4 default | Why it changed |
+|-----|--------------------|--------------------|----------------|
+| `fs.s3a.connection.maximum` | 96 | 500 | HTTP connection pool resized for
the AWS SDK v2 client and the larger thread pool, avoiding pool starvation
under parallel uploads. |
+| `fs.s3a.threads.max` | 10 | 96 | Sized to match the larger connection pool
for parallel upload and copy operations. |
Review Comment:
This 3.3 column is from `Constants.java`, but `core-default.xml` wins: Flink
currently runs with 64 threads, 20 attempts and a 5s establish timeout. The
restore example below sets values nobody had.
##########
flink-filesystems/flink-s3-fs-presto/pom.xml:
##########
@@ -56,13 +56,44 @@ under the License.
<artifactId>flink-architecture-tests-test</artifactId>
<scope>test</scope>
</dependency>
-
+
<!-- S3 base (bundled) -->
<dependency>
<groupId>org.apache.flink</groupId>
<artifactId>flink-s3-fs-base</artifactId>
<version>${project.version}</version>
<optional>${flink.markBundledAsOptional}</optional>
+ <exclusions>
+ <!-- Exclude all AWS SDK v2 dependencies -
Presto uses SDK v1 -->
Review Comment:
The Presto jar still bundles hadoop-aws 3.4.3 and analyticsaccelerator-s3,
which need SDK v2, plus hadoop-common's Netty tree. Nothing else in the jar
uses them, so they could be excluded here as well.
##########
flink-filesystems/flink-fs-hadoop-shaded/src/main/resources/META-INF/NOTICE:
##########
@@ -9,39 +9,46 @@ This project bundles the following dependencies under the
Apache Software Licens
- com.fasterxml.jackson.core:jackson-annotations:2.22
- com.fasterxml.jackson.core:jackson-core:2.22.2
- com.fasterxml.jackson.core:jackson-databind:2.22.2
-- com.fasterxml.woodstox:woodstox-core:5.3.0
-- com.google.guava:failureaccess:1.0
-- com.google.guava:guava:27.0-jre
+- com.fasterxml.woodstox:woodstox-core:5.4.0
+- com.google.guava:failureaccess:1.0.1
+- com.google.guava:guava:32.0.1-jre
- com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava
-- com.google.j2objc:j2objc-annotations:1.1
-- commons-beanutils:commons-beanutils:1.9.4
-- commons-collections:commons-collections:3.2.2
+- com.google.j2objc:j2objc-annotations:2.8
- commons-io:commons-io:2.15.1
- commons-logging:commons-logging:1.1.3
+- io.dropwizard.metrics:metrics-core:3.2.5
+- org.apache.commons:commons-collections4:4.4
- org.apache.commons:commons-compress:1.26.0
-- org.apache.commons:commons-configuration2:2.1.1
+- org.apache.commons:commons-configuration2:2.10.1
- org.apache.commons:commons-lang3:3.18.0
- org.apache.commons:commons-text:1.10.0
-- org.apache.hadoop.thirdparty:hadoop-shaded-guava:1.1.1
-- org.apache.hadoop.thirdparty:hadoop-shaded-protobuf_3_7:1.1.1
-- org.apache.hadoop:hadoop-annotations:3.3.4
-- org.apache.hadoop:hadoop-auth:3.3.4
-- org.apache.hadoop:hadoop-common:3.3.4
-- org.apache.kerby:kerb-core:1.0.1
-- org.apache.kerby:kerby-asn1:1.0.1
-- org.apache.kerby:kerby-pkix:1.0.1
-- org.apache.kerby:kerby-util:1.0.1
+- org.apache.hadoop.thirdparty:hadoop-shaded-guava:1.5.0
+- org.apache.hadoop.thirdparty:hadoop-shaded-protobuf_3_25:1.5.0
+- org.apache.hadoop:hadoop-annotations:3.4.3
+- org.apache.hadoop:hadoop-auth:3.4.3
+- org.apache.hadoop:hadoop-common:3.4.3
+- org.apache.kerby:kerb-core:2.0.3
+- org.apache.kerby:kerb-crypto:2.0.3
+- org.apache.kerby:kerb-util:2.0.3
+- org.apache.kerby:kerby-asn1:2.0.3
+- org.apache.kerby:kerby-config:2.0.3
+- org.apache.kerby:kerby-pkix:2.0.3
+- org.apache.kerby:kerby-util:2.0.3
+- org.codehaus.jettison:jettison:1.5.4
- org.xerial.snappy:snappy-java:1.1.10.7
This project bundles the following dependencies under the MIT
(https://opensource.org/licenses/MIT)
-- org.checkerframework:checker-qual:2.5.2
-- org.codehaus.mojo:animal-sniffer-annotations:1.17
+- org.checkerframework:checker-qual:3.33.0
This project bundles the following dependencies under BSD-2 License
(https://opensource.org/licenses/BSD-2-Clause).
See bundled license files for details.
-- dnsjava:dnsjava:2.1.7
+- dnsjava:dnsjava:3.6.1
+
+This project bundles the following dependencies under the Bouncy Castle
License (https://www.bouncycastle.org/licence.html)
+
+- org.bouncycastle:bcprov-jdk18on:1.82
Review Comment:
Nothing else in this jar references `org.bouncycastle` (checked with jdeps),
so it could be excluded like Netty. It adds 4.5k classes to every Hadoop-based
plugin.
##########
flink-filesystems/flink-s3-fs-base/src/main/java/org/apache/flink/fs/s3/common/writer/S3RecoverableSerializer.java:
##########
@@ -46,22 +53,24 @@ private S3RecoverableSerializer() {}
@Override
public int getVersion() {
- return 1;
+ return 2;
Review Comment:
This changes checkpointed state, so "serializers" in the description is a
yes (the v1 fixture does match master's bytes). I think it deserves its own
commit, separate from the bump.
##########
flink-filesystems/flink-s3-fs-base/pom.xml:
##########
@@ -32,6 +32,10 @@ under the License.
<properties>
<fs.s3.aws.version>1.12.779</fs.s3.aws.version>
+ <!-- Held above Hadoop 3.4.3's bundled aws-java-sdk-v2 because
hadoop-aws 3.4.3's
+ ChecksumSupport.<clinit> references CRC64_NVME, which was
added to
+ software.amazon.awssdk:checksums-spi in 2.30.x. Aligned
with flink-s3-fs-native. -->
+ <fs.s3.aws.sdk2.version>2.44.4</fs.s3.aws.sdk2.version>
Review Comment:
Hadoop 3.4.3 builds against SDK 2.35.4, which already has `CRC64_NVME`, so
this reason doesn't hold. Why deviate from the version Hadoop tested, other
than aligning with flink-s3-fs-native?
##########
flink-filesystems/pom.xml:
##########
@@ -34,7 +34,7 @@ under the License.
<packaging>pom</packaging>
<properties>
- <fs.hadoopshaded.version>3.3.4</fs.hadoopshaded.version>
+ <fs.hadoopshaded.version>3.4.3</fs.hadoopshaded.version>
Review Comment:
This also moves the Azure, OSS and GCS plugins to Hadoop 3.4.3, and their
credential-bound tests were skipped here. How were those verified? The
description only covers S3.
##########
flink-filesystems/flink-s3-fs-hadoop/pom.xml:
##########
@@ -284,14 +315,27 @@ under the License.
</excludes>
</filter>
<filter>
-
<artifact>com.amazonaws:aws-java-sdk-s3</artifact>
- <!--
Make sure we are using the overridden XmlResponsesSaxParser of flink-s3-fs-base.
-
Filter must be removed as soon as XmlResponsesSaxParser of this module is
-
dropped, for example when discontinuing support for Java 8. -->
+
<artifact>org.apache.flink:flink-s3-fs-base</artifact>
+ <!--
Exclude XmlResponsesSaxParser from flink-s3-fs-base as it is AWS SDK v1 code.
+
We do not need this for SDK v2. This is only needed by Presto. -->
<excludes>
<exclude>com/amazonaws/services/s3/model/transform/XmlResponsesSaxParser**</exclude>
</excludes>
</filter>
+ <filter>
+ <!--
analyticsaccelerator-s3 (transitive of hadoop-aws 3.4.x) ships
+
LICENSE, NOTICE, and THIRD-PARTY-NOTICES at the jar root. The
+
license checker rejects root-level license files; the
+
THIRD-PARTY-NOTICES also references GPL/LGPL terms that the
+
checker flags. Attribution is consolidated in this module's
+
META-INF/NOTICE. -->
+
<artifact>software.amazon.s3.analyticsaccelerator:analyticsaccelerator-s3</artifact>
+
<excludes>
+
<exclude>LICENSE</exclude>
+
<exclude>NOTICE</exclude>
+
<exclude>THIRD-PARTY-NOTICES</exclude>
Review Comment:
analyticsaccelerator-s3 shades Caffeine, parquet-format 2.10.0 with
libthrift, and failsafe 3.3.2, but the NOTICE doesn't list them. If its own
notices are dropped, they need nested entries like hadoop-shaded-guava.
##########
flink-filesystems/flink-fs-hadoop-shaded/pom.xml:
##########
@@ -296,13 +330,6 @@ under the License.
</excludes>
</filter>
</filters>
- <transformers
combine.children="append">
- <transformer
implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
-
<manifestEntries>
-
<Multi-Release>true</Multi-Release>
Review Comment:
Removing the `Multi-Release` entry (FLINK-40084) breaks the Java 21 nightly
profile: dnsjava 3.6.1's resolver SPI class only exists under
`META-INF/versions/18`, so the OSS, Azure and GCS tests fail discovery.
Restoring it fixes that.
##########
flink-filesystems/flink-fs-hadoop-shaded/pom.xml:
##########
@@ -78,6 +84,19 @@ under the License.
<groupId>com.sun.jersey</groupId>
<artifactId>jersey-server</artifactId>
</exclusion>
+ <!-- Pulled in by hadoop-common 3.4.x via
com.github.pjfanning:jersey-json,
+ ships com/sun/jersey/json/** classes. -->
+ <exclusion>
+ <groupId>com.github.pjfanning</groupId>
+ <artifactId>jersey-json</artifactId>
+ </exclusion>
+ <!-- Transitive of jersey-json above; ships
com/sun/xml/bind/** classes
+ (CDDL+GPLv2-with-classpath-exception). The
JDK provides JAXB RI; we
Review Comment:
Nothing in the shaded jar references `com.sun.xml.bind` (only `HttpServer2`
touches Jersey), so the exclusion is fine. The JDK dropped JAXB in 11 though,
so this comment is off; same in flink-s3-fs-base.
##########
flink-filesystems/flink-s3-fs-hadoop/src/main/resources/META-INF/NOTICE:
##########
@@ -3,56 +3,107 @@ Copyright 2014-2026 The Apache Software Foundation
This project bundles the following dependencies under the Apache Software
License 2.0 (http://www.apache.org/licenses/LICENSE-2.0.txt)
-- com.amazonaws:aws-java-sdk-core:1.12.779
-- com.amazonaws:aws-java-sdk-dynamodb:1.12.779
-- com.amazonaws:aws-java-sdk-kms:1.12.779
-- com.amazonaws:aws-java-sdk-s3:1.12.779
-- com.amazonaws:aws-java-sdk-sts:1.12.779
-- com.amazonaws:jmespath-java:1.12.779
- com.fasterxml.jackson.core:jackson-annotations:2.22
- com.fasterxml.jackson.core:jackson-core:2.22.2
- com.fasterxml.jackson.core:jackson-databind:2.22.2
-- com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.22.2
-- com.fasterxml.woodstox:woodstox-core:5.3.0
-- com.google.guava:failureaccess:1.0
-- com.google.guava:guava:27.0-jre
+- com.fasterxml.woodstox:woodstox-core:5.4.0
+- com.google.guava:failureaccess:1.0.1
+- com.google.guava:guava:32.0.1-jre
- com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava
-- com.google.j2objc:j2objc-annotations:1.1
-- commons-beanutils:commons-beanutils:1.9.4
+- com.google.j2objc:j2objc-annotations:2.8
- commons-codec:commons-codec:1.15
-- commons-collections:commons-collections:3.2.2
- commons-io:commons-io:2.15.1
- commons-logging:commons-logging:1.1.3
-- joda-time:joda-time:2.5
+- io.dropwizard.metrics:metrics-core:3.2.5
+- io.netty:netty-buffer:4.2.17.Final
Review Comment:
These Netty modules embed third-party code (jctools, hpack, jzlib and more)
whose licenses flink-shaded-netty ships under `META-INF/licenses`. This jar
ships none of them; I think they need to come along.
--
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]
