Skip to content
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

[HUDI-2191] Bump flink version to 1.13.1 #3291

Merged
merged 1 commit into from
Aug 16, 2021

Conversation

danny0405
Copy link
Contributor

Tips

What is the purpose of the pull request

(For example: This pull request adds quick-start document.)

Brief change log

(for example:)

  • Modify AnnotationLocation checkstyle rule in checkstyle.xml

Verify this pull request

(Please pick either of the following options)

This pull request is a trivial rework / code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end.
  • Added HoodieClientWriteTest to verify the change.
  • Manually verified the change by running a job locally.

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

@danny0405 danny0405 added priority:blocker flink Issues related to flink labels Jul 18, 2021
@hudi-bot
Copy link

hudi-bot commented Jul 18, 2021

CI report:

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

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2021

Codecov Report

Merging #3291 (e8295bb) into master (572a214) will increase coverage by 0.01%.
The diff coverage is 59.25%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3291      +/-   ##
============================================
+ Coverage     47.83%   47.84%   +0.01%     
- Complexity     5565     5567       +2     
============================================
  Files           936      936              
  Lines         41663    41662       -1     
  Branches       4197     4195       -2     
============================================
+ Hits          19929    19933       +4     
+ Misses        19960    19955       -5     
  Partials       1774     1774              
Flag Coverage Δ
hudicli 39.97% <ø> (ø)
hudiclient 34.52% <ø> (ø)
hudicommon 48.71% <ø> (+0.04%) ⬆️
hudiflink 59.30% <59.25%> (-0.06%) ⬇️
hudihadoopmr 52.02% <ø> (ø)
hudisparkdatasource 67.33% <ø> (+0.05%) ⬆️
hudisync 55.97% <ø> (ø)
huditimelineservice 64.07% <ø> (ø)
hudiutilities 59.77% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../org/apache/hudi/streamer/HoodieFlinkStreamer.java 0.00% <ø> (ø)
...in/java/org/apache/hudi/table/HoodieTableSink.java 10.34% <16.66%> (+0.34%) ⬆️
...ache/hudi/sink/StreamWriteOperatorCoordinator.java 66.85% <50.00%> (-2.11%) ⬇️
.../java/org/apache/hudi/table/HoodieTableSource.java 56.64% <71.42%> (ø)
...org/apache/hudi/sink/event/WriteMetadataEvent.java 71.92% <100.00%> (ø)
...he/hudi/sink/partitioner/BucketAssignFunction.java 80.00% <100.00%> (ø)
...java/org/apache/hudi/table/HoodieTableFactory.java 86.84% <100.00%> (ø)
...e/hudi/common/table/log/HoodieLogFormatWriter.java 79.68% <0.00%> (+1.56%) ⬆️
...in/scala/org/apache/hudi/HoodieStreamingSink.scala 29.60% <0.00%> (+1.60%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 572a214...e8295bb. Read the comment docs.

@garyli1019
Copy link
Member

@danny0405 Most users do not have 1.13 in their production environment I believe. What benefit of this upgrade brings to us?

Copy link
Member

@garyli1019 garyli1019 left a comment

Choose a reason for hiding this comment

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

I don't think we should do this upgrade before the release unless we do a better job on version compatibility.

@danny0405
Copy link
Contributor Author

danny0405 commented Jul 19, 2021

The Amazon AWS EMR and Aliyun EMR next version all need this patch for the flink hudi integration.

@garyli1019
Copy link
Member

Is there any way we can make Hudi compatible with both Flink 1.12 and 1.13? Many users still have <1.12 in their production environment AFAIK.

@danny0405
Copy link
Contributor Author

Is there any way we can make Hudi compatible with both Flink 1.12 and 1.13? Many users still have <1.12 in their production environment AFAIK.

The Flink interface compatibility between minor version is a shit, not to say major version, it's hard to be compatible for both 1.12 and 1.13.

I guess the flink compatibility would be better from 1.14.

@garyli1019
Copy link
Member

@danny0405 I think we should at least support 1.12 for the 0.9.0 release, if you think this upgrade is necessary, I'd recommend support both 1.12 and 1.13. Maybe we can have a V1 and V2 connector?

@danny0405
Copy link
Contributor Author

@danny0405 I think we should at least support 1.12 for the 0.9.0 release, if you think this upgrade is necessary, I'd recommend support both 1.12 and 1.13. Maybe we can have a V1 and V2 connector?

I'm planning to upgrade the version to 1.12.3 for 0.9.0 release and after the release upgrade it to 1.13.1.

@garyli1019
Copy link
Member

@danny0405 What's the impact of upgrading from 1.12.2 to 1.12.3? we should be very cautious about the version upgrade unless they are fully backward compatible.

@haormj
Copy link

haormj commented Jul 27, 2021

@garyli1019 something are not work well, like pulsar sql connector

https://search.maven.org/artifact/io.streamnative.connectors/pulsar-flink-sql-connector_2.11

@garyli1019
Copy link
Member

@garyli1019 something are not work well, like pulsar sql connector

https://search.maven.org/artifact/io.streamnative.connectors/pulsar-flink-sql-connector_2.11

Hi @haormj , what do you mean something is not working well? Do you mean you need Flink 1.13 to use the Flink pulsar connector?

@haormj
Copy link

haormj commented Jul 27, 2021

@garyli1019 now I use 1.12.2 + hudi master, but I can not use pulsar sql connector, I suspect it was caused by flink 1.12.2, and I attempt to merge this change and retry it

I just found pulsar-flink-sql-connector 1.12.3, 1.12.4 and 1.13.1

@haormj
Copy link

haormj commented Jul 28, 2021

@garyli1019 hudi + flink 1.13.1 + pulsar-flink-sql-connector_2.11-1.13.1.1.jar works well, but I not try 1.12.3, 1.12.4

@vinothchandar vinothchandar added this to Ready for Review in PR Tracker Board Aug 10, 2021
Copy link
Contributor

@swuferhong swuferhong left a comment

Choose a reason for hiding this comment

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

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flink Issues related to flink
Projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants