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.

Sign up for GitHub

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

Jump to bottom

[AMORO-1983] Upgrading Flink 1.17 based on Mixed-format table #2001

Merged
merged 19 commits into from
Oct 11, 2023
Merged

[AMORO-1983] Upgrading Flink 1.17 based on Mixed-format table #2001

merged 19 commits into from
Oct 11, 2023

Conversation

czy006
Copy link
Contributor

@czy006 czy006 commented Sep 19, 2023 •
edited
Loading

Why are the changes needed?

Close #1983.

Brief change log

  • Copy Flink 1.15 Code to Flink-Common Mode And Flink-Common-format
  • Support Version Flink 1.17 and
  • Fix Wrong Test Api
  • Remove KafkaTestBase （Just One Kafka Test Util）

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 type:build module:ams-dashboard Ams dashboard module labels Sep 19, 2023
@codecov
Copy link

codecov bot commented Sep 19, 2023 •
edited
Loading

Codecov Report

All modified lines are covered by tests ✅

see 14 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@czy006
Copy link
Contributor Author

czy006 commented Sep 19, 2023

image image

These are some errors during unit testing. It can be seen that some packages are missing.

org.apache.calcite.rel.metadata.DefaultRelMetadataProvider

It comes from flink-table-planner_2.12-1.17 I confirmed that it has been imported but there is still an error. I am looking for the reason.

@YesOrNo828
Copy link
Contributor

Added Spotless plugin to Flink module via PR, @czy006. Please review and add to the flink-common and flink-1.17 modules.

@czy006
Copy link
Contributor Author

czy006 commented Sep 27, 2023 •
edited
Loading

I found some strange phenomena when compiling locally:

When I run a single test case in local idea, it can pass completely, as shown below

image

When I ran the test command on maven to conduct all tests, it threw an exception. For example, the following file said that it could not find a class related to flink-table.

com.netease.arctic.flink.table.TestUnkeyed.txt

I don't know what's going on. I've been trying to troubleshoot for a long time but haven't found the problem. Can you guys take a look?

cc @YesOrNo828 @zhoujinsong @baiyangtx

@czy006
Copy link
Contributor Author

czy006 commented Sep 28, 2023

Added Spotless plugin to Flink module via PR, @czy006. Please review and add to the flink-common and flink-1.17 modules.

Get It

@czy006
Copy link
Contributor Author

czy006 commented Oct 9, 2023 •
edited
Loading

image

Use Quick Start Mixed Format Demo with Flink Local Test（Streamming Job）

@czy006 czy006 changed the title [Draft][AMORO-1983] Upgrading Flink 1.17 based on Mixed-format table [AMORO-1983] Upgrading Flink 1.17 based on Mixed-format table Oct 9, 2023
zhoujinsong
zhoujinsong reviewed Oct 9, 2023
View reviewed changes
Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

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

@czy006 Thanks a lot for your work!

I left some commets.
BTW, we may need to maintain additional integration test cases for each Flink version. At this stage, we can select appropriate test cases from "common" and migrate them over. What do you think?

@czy006 czy006 force-pushed the issues/amoro-1983 branch from 8ff161b to e61b90a Compare October 10, 2023 02:34
@czy006
Copy link
Contributor Author

czy006 commented Oct 10, 2023

@czy006 Thanks a lot for your work!

I left some commets. BTW, we may need to maintain additional integration test cases for each Flink version. At this stage, we can select appropriate test cases from "common" and migrate them over. What do you think?

We should differentiate between these unit tests. After communication, I think some unit tests should be integrated under each Flink version to ensure that the functions of the current version can be covered

@github-actions github-actions bot removed the module:ams-dashboard Ams dashboard module label Oct 10, 2023
@czy006 czy006 force-pushed the issues/amoro-1983 branch from 1136ae7 to b9acc16 Compare October 10, 2023 14:19
@czy006
Copy link
Contributor Author

czy006 commented Oct 11, 2023

after discuss ,patch #1994 feature in flink 1.17

@czy006 czy006 force-pushed the issues/amoro-1983 branch from bcbc4aa to 42f1b59 Compare October 11, 2023 04:08
zhoujinsong
zhoujinsong approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

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

LGTM.

@zhoujinsong zhoujinsong merged commit 62cf908 into apache:master Oct 11, 2023
@zhoujinsong zhoujinsong mentioned this pull request Oct 12, 2023
Merged
3 tasks
ShawHee pushed a commit to ShawHee/arctic that referenced this pull request Dec 29, 2023
@czy006 @zhoujinsong @ShawHee
[AMORO-1983] Upgrading Flink 1.17 based on Mixed-format table (apache…
cf6de88 
…#2001)

* [AMORO-1983][Feature] Copy From Flink 1.15 Code to Flink 1.17

* [AMORO-1983][Feature] Flink 1.17 API compatible

* [AMORO-1983][Feature] Copy From Flink 1.15 Test Code to Flink 1.17 Test Code

* [AMORO-1983][fix] fix package rename to planner

* [AMORO-1983][Feature] Flink 1.17 Mode Test Fix

* [AMORO-1983][Feature] fix pom

* [AMORO-1983] FlinkSplitPlanner Remove SequenceNumber

* [AMORO-1983] TestUnkeyed Replace to KafkaContainer Test

* Add KafkaContainerTest CountAllRecords

* Remove KafkaTestBase

* spotless code

* fix TestCatalog use default after drop catalog

* fix LookupITCase set streaming true

* fix checkstyle

* fix test timeout thread commit failed

* remove bridge to public common

* fix pom problem

* patch apache#1940

---------

Co-authored-by: ZhouJinsong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhoujinsong zhoujinsong zhoujinsong approved these changes

@YesOrNo828 YesOrNo828 Awaiting requested review from YesOrNo828

@baiyangtx baiyangtx Awaiting requested review from baiyangtx

Assignees
No one assigned
Labels
module:mixed-flink Flink moduel for Mixed Format type:build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature]: Upgrading Flink version based on Mixed-format table
3 participants
@czy006 @YesOrNo828 @zhoujinsong