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

[Improvement]: Upgrade to iceberg 1.6.0 (#3084) #3071

Closed
wants to merge 2 commits into from

Conversation

czy006
Copy link
Contributor

@czy006 czy006 commented Jul 30, 2024

Why are the changes needed?

#3084.

Brief change log

  • Remove Flink 1.15/1.16/1.17 Mixed Format Support
  • Mixed Format Flink 1.18 Base Support Iceberg 1.6.X
  • Upgrade iceberg to 1.6.X
  • Fix some Mixed Format Unit Test Failed

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible
  • Add screenshots for manual tests if appropriate
  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (yes)
  • If yes, how is the feature documented? (not documented)

@github-actions github-actions bot added module:mixed-flink Flink moduel for Mixed Format module:mixed-spark Spark module for Mixed Format module:core Core module type:build labels Jul 30, 2024
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ConradJam seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@czy006 czy006 force-pushed the iceberg-1.6.0 branch 2 times, most recently from a638b49 to cd48a10 Compare August 15, 2024 02:33
@czy006
Copy link
Contributor Author

czy006 commented Aug 15, 2024

@czy006 czy006 changed the title [Draft]: Upgrade Iceberg to 1.6.0 [Draft]: Upgrade Iceberg to 1.6.0(#3084) Aug 21, 2024
@github-actions github-actions bot removed the module:core Core module label Aug 27, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 23.17%. Comparing base (9def89f) to head (47781b0).
Report is 8 commits behind head on master.

Files Patch % Lines
...java/org/apache/amoro/table/BasicUnkeyedTable.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3071      +/-   ##
============================================
- Coverage     27.64%   23.17%   -4.47%     
+ Complexity     2496     2330     -166     
============================================
  Files           360      368       +8     
  Lines         37133    37914     +781     
  Branches       5372     5456      +84     
============================================
- Hits          10265     8787    -1478     
- Misses        25908    28353    +2445     
+ Partials        960      774     -186     
Flag Coverage Δ
trino 23.17% <0.00%> (-4.47%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@czy006 czy006 marked this pull request as ready for review August 28, 2024 08:47
@czy006 czy006 changed the title [Draft]: Upgrade Iceberg to 1.6.0(#3084) [Improvement]: Upgrade to iceberg 1.6.0 (#3084) Aug 28, 2024
@czy006 czy006 force-pushed the iceberg-1.6.0 branch 2 times, most recently from 805b425 to 10bc289 Compare August 29, 2024 02:55
@czy006
Copy link
Contributor Author

czy006 commented Aug 29, 2024

ops,some time ci was blocker have long time.Maybe we should check

@baiyangtx
Copy link
Contributor

It looks like there will be many engine versions that will no longer be supported.

@zhoujinsong PTAL

@zhoujinsong
Copy link
Contributor

Maybe we should follow Iceberg 1.6, support:

  • Flink 1.17/1.18/1.19
  • Spark 3.3/3.4/3.5

What do you think?

@czy006
Copy link
Contributor Author

czy006 commented Sep 2, 2024

Details

This will be supported in the future PR, Flink 1.20 as the LTS long-term support version, I recommend as the default version option.

Maybe we should follow Iceberg 1.6, support:

  • Flink 1.17/1.18/1.19
  • Spark 3.3/3.4/3.5

What do you think?

This will be support in the future PR, Flink 1.20 as the LTS long-term support version, I recommend as the default version option.Now Default Spark is 3.3,Flink is 1.18

@czy006
Copy link
Contributor Author

czy006 commented Sep 4, 2024

image

Optimizing Task Run Normal

@czy006 czy006 requested a review from XBaith September 4, 2024 02:25
@zhongqishang
Copy link
Contributor

zhongqishang commented Sep 5, 2024

I think it is not a good idea to remove so many versions support.
What is the reason for deleting the support? Is it just that the official does not support it?

Maybe we can keep the combination of old flink and old iceberg versions. @zhoujinsong @baiyangtx @czy006

And It is best to consult users on the mailing list or wechat group and make unsupported plans.

@czy006
Copy link
Contributor Author

czy006 commented Sep 5, 2024

I think it is not a good idea to remove so many versions support. What is the reason for deleting the support? Is it just that the official does not support it?

Maybe we can keep the combination of old flink and old iceberg versions. @zhoujinsong @baiyangtx @czy006

mixed format relies on the spark and flink versions, and currently the iceberg community only supports the latest 3 versions. For example, they have to give up spark 3.2 their support.the last support in 1.4.3, iceberg if we upgrade version, we must give up its support it.flink version seems(I will add flink1.19 1.20 in the next pr),Spark now just support 3.3

@czy006
Copy link
Contributor Author

czy006 commented Sep 18, 2024

I have initiated relevant discussions on the email list:
https://lists.apache.org/thread/1crbstg7prjxz8xokd7dtj1o5mgv1034

Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@amoro.apache.org list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Oct 19, 2024
Copy link

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:mixed-flink Flink moduel for Mixed Format module:mixed-spark Spark module for Mixed Format stale type:build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants