-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-53927][BUILD][DSTREAM][4.0] Upgrade kinesis client and fix kinesis integration tests #52654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: branch-4.0
Are you sure you want to change the base?
Conversation
…integration tests (ENABLE_KINESIS_TESTS=1)
|
@sarutak Please review. |
|
Also, please add the target branch to the title like |
| </dependency> | ||
| <dependency> | ||
| <groupId>com.amazonaws</groupId> | ||
| <groupId>software.amazon.kinesis</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry but we don't expect this kind of dramatic dependency changes on the release branches, @vrozov .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dongjoon-hyun This is test only dependency and the old dependency does not work on 4.x.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dongjoon-hyun
Do you have any concern even though amazon-kinesis-producer is a test dependency and affects only kinesis-asl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dongjoon-hyun @sarutak Note that the entire impact of the change is limited to KPLBasedKinesisTestUtils.scala and it is necessary due to other Spark dependencies being upgraded making it incompatible with the existing Kinesis producer library version.
|
We had better focus on delivering Apache Spark 4.1.0
|
|
@dongjoon-hyun The problem is that Kinesis integration is broken on branch-4.0 and my testing also shows that it is broken on branch-3.5 as well (double checking). The issue impacts not only tests. Running tests against actual cluster or demo application do not work. Sorry, I don't fully understand how fixing this regression bug on branch-4.0 impacts Spark 4.1 release. |
|
@sarutak Please take a look. Spark SBT build required a workaround. |
| <groupId>software.amazon.awssdk</groupId> | ||
| <artifactId>sts</artifactId> | ||
| <version>${aws.java.sdk.v2.version}</version> | ||
| <scope>test</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to add these dependency for 4.0 even though they are not needed for 4.1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sarutak @dongjoon-hyun The issue is caused by 4.0 dependency on hadoop-aws:3.4.1 that was upgraded to 3.4.2 in 4.1. hadoop-aws:3.4.1 has transitive dependency on AWS Java SDK v2 2.24.6 that is not compatible with Kinesis Producer transitive dependency on AWS Java SDK v2 2.29.24. In general, Spark should be enforcing specific version of AWS Java SDK v2 (the one specified for aws.java.sdk.v2.version) instead of relying on transitive dependencies that may not be compatible with each other.
Note that this is not an issue for maven build as it correctly handles transitive dependencies, and causes the issue that you noticed in SBT.
What changes were proposed in this pull request?
Backport of #52630 to 4.0
Why are the changes needed?
branch-4.0 is subject to the same error as master. The integration with kinesis does not work due to dependencies conflict.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Using AWS credentials and
ENABLE_KINESIS_TESTS=1 build/mvn test -Pkinesis-asl -pl connector/kinesis-aslWas this patch authored or co-authored using generative AI tooling?
No