Skip to content

Conversation

@guptashailesh92
Copy link
Contributor

Change Logs

Upgrade hudi flink connector to 1.20.0

Impact

Added hudi flink connector for 1.20.0.

Risk level (write none, low medium or high below)

medium

Documentation Update

Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".

none

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@github-actions github-actions bot added the size:XL PR with lines of changes > 1000 label Sep 19, 2024
@danny0405 danny0405 self-assigned this Sep 19, 2024
@guptashailesh92
Copy link
Contributor Author

@danny0405 please let me know if anything else is required here.

@danny0405
Copy link
Contributor

@guptashailesh92 Thanks for the contribution, have you already referenced #11779 for the changes? Especially we need to upload the docker images for bundle validation tests, only Hudi PMC got permission do do this now.

@guptashailesh92
Copy link
Contributor Author

Yes, i used the referenced cr: #11779 for raising this CR.

<flink.avro.version>1.11.1</flink.avro.version>
<flink.format.parquet.version>1.13.1</flink.format.parquet.version>
<flink.connector.kafka.version>3.2.0-1.19</flink.connector.kafka.version>
<!-- check kafka version -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the discussion regarding flink connector kafka version to 1.20, but it has not been release yet.
apache/flink-connector-kafka#111

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it.

Copy link
Contributor

@CTTY CTTY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note:

  • hudi-flink1.20.x is a copy of existing hudi-flink1.19.x with only version changes, there is no changes made to adapt to Flink upgrade.
  • This change has passed EMR Hudi integtest.

Left some minor comments around avro version

"-Dscala-2.12 -Dflink1.17 -Davro.version=1.11.1 -pl packaging/hudi-flink-bundle -am"
"-Dscala-2.12 -Dflink1.18 -Davro.version=1.11.1 -pl packaging/hudi-flink-bundle -am"
"-Dscala-2.12 -Dflink1.19 -Davro.version=1.11.1 -pl packaging/hudi-flink-bundle -am"
"-Dscala-2.12 -Dflink1.20 -Davro.version=1.11.1 -pl packaging/hudi-flink-bundle -am"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avro version here should be 1.11.3

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated. thanks for the review.

pom.xml Outdated
<hudi.flink.module>hudi-flink1.20.x</hudi.flink.module>
<flink.bundle.version>1.20</flink.bundle.version>
<!-- This is fixed to match with version from flink-avro -->
<flink.avro.version>1.11.1</flink.avro.version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update this avro version as well since we are changing the default flink version to 1.20

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

Copy link
Contributor

@yihua yihua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've built and uploaded the following bundle validation docker images to the Docker Hub, based on the scripts in this PR:

yihua
yihua previously requested changes Oct 8, 2024
HIVE_VERSION=3.1.3
DERBY_VERSION=10.14.1.0
FLINK_VERSION=1.19.0
FLINK_VERSION=1.20.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image tag IMAGE_TAG needs to be updated to properly test Hudi Flink bundle on the latest Flink version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

@danny0405
Copy link
Contributor

@guptashailesh92 Hi, the TestHoodieFlinkQuickstart.testHoodieFlinkQuickstart has been failing while swithing to 1.20 release, I have debug it a little, here is the info I can share:

  1. local IDEA run can pass successfully;
  2. use cmd mvn test -Punit-tests -D"scala-2.12" -D"flink1.20" -pl hudi-examples/hudi-examples-flink to trigger the test can reproduce the error;
  3. I enabled the -Dmaven.surefire.debug for the mvn cmd and start a remote debug, and finds out that there are fatal errors arised by the RPC calls, the mini-cluster was indeed closed before the job was submitted.

But I still didn't find the root cause, maybe you can spend with your spare time to do some research.

@danny0405
Copy link
Contributor

@guptashailesh92 I have addressed the issues of the test failure, it is because we have conflict dependency of reload4j in the classpath, it includes the legacy slf4j MDC adapter Reload4jMDCAdapter which triggers the NPE for invocation MDC.setContextMap, the NPE then causes the Flink resource manager to crush then the mini-cluster is shutdown.

The solution is to remove the reload4j related jars because we already have log4j-1.2-api jar dependency in module hudi-tests-common.

@guptashailesh92
Copy link
Contributor Author

thank you @danny0405, I am little occupied with things, haven't been able to spend much time.

@hudi-bot
Copy link
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@danny0405 danny0405 added the engine:flink Flink integration label Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine:flink Flink integration size:XL PR with lines of changes > 1000

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

5 participants