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

[vitess] Add vitess cdc connector #456

Closed
wants to merge 20 commits into from
Closed

[vitess] Add vitess cdc connector #456

wants to merge 20 commits into from

Conversation

s-gelazevicius
Copy link
Contributor

This is initial implementation of Vitess CDC connector.
For a fully detailed documentation of Vitess Debezium Connector , please see Documentation.

@leonardBang
Copy link
Contributor

Thanks @s-gelazevicius for the great contribution, I'm not familiar with Vitess, but I'm willing to learn it.
Could you share more about the background of vitess-cdc connector? e.g. How is it used in your business/company, the data-scale etc.

@leonardBang leonardBang self-requested a review September 23, 2021 10:20
@s-gelazevicius
Copy link
Contributor Author

s-gelazevicius commented Sep 23, 2021 •
edited
Loading

Hi @leonardBang

Thanks @s-gelazevicius for the great contribution, I'm not familiar with Vitess, but I'm willing to learn it.
Could you share more about the background of vitess-cdc connector? e.g. How is it used in your business/company, the data-scale etc.

Vitess is clustering solution on top of Mysql. It automatically handles sharding and has some other nice features. From client perspective same Mysql JDBC driver can be used.

Here in Vinted, databases are quite large and we used Mysql directly, but it was hard to scale, sharding required a lot of manual work. So now we are migrating to Vitess.

This connector will be used for our CDC ingestion pipeline implemented as a flink application.

@MartijnVisser
Copy link
Contributor

Thanks for the PR @s-gelazevicius !

@leonardBang
Copy link
Contributor

Hi, @s-gelazevicius

Here in Vinted, databases are quite large and we used Mysql directly, but it was hard to scale, sharding required a lot of manual work. So now we are migrating to Vitess.
This connector will be used for our CDC ingestion pipeline implemented as a flink application.

Thanks for your detailed explanation, happy to see the developer from Vinted and welcome to community, I'll help review this PR.

@leonardBang leonardBang mentioned this pull request Dec 15, 2021
Closed
17 tasks
@s-gelazevicius s-gelazevicius force-pushed the feature/add-vitess-connector branch 2 times, most recently from aa9a886 to ec0c5bc Compare March 16, 2022 13:36
@s-gelazevicius
Copy link
Contributor Author

Hi, @leonardBang any updates on this pull request. I have re-based it from the latest master to keep it fresh. It's been siting here for a long time. It would be really nice to have this in upstream.

@leonardBang
Copy link
Contributor

@s-gelazevicius Thanks for the great work and BIG sorry for my late response. I'm busy on supporting the TiDB and OceanBase vendors, I'll review this PR soon after the OceanBase PR review finished.

fsk119
fsk119 reviewed Mar 18, 2022
View reviewed changes
Copy link
Member

@fsk119 fsk119 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I left some comments.

...-sql-connector-postgres-cdc/src/main/java/com/ververica/cdc/connectors/vitess/DummyDocs.java Outdated Show resolved Hide resolved
...r-vitess-cdc/src/main/java/com/ververica/cdc/connectors/vitess/table/VitessTableFactory.java Show resolved Hide resolved
...r-vitess-cdc/src/main/java/com/ververica/cdc/connectors/vitess/table/VitessTableFactory.java Show resolved Hide resolved
...r-vitess-cdc/src/main/java/com/ververica/cdc/connectors/vitess/table/VitessTableFactory.java Outdated Show resolved Hide resolved
...or-vitess-cdc/src/main/java/com/ververica/cdc/connectors/vitess/table/VitessTableSource.java Outdated Show resolved Hide resolved
...itess-cdc/src/test/java/com/vervetica/cdc/connectors/vitess/table/VitessConnectorITCase.java Show resolved Hide resolved
...connector-vitess-cdc/src/test/java/com/vervetica/cdc/connectors/vitess/VitessSourceTest.java Show resolved Hide resolved
...k-connector-vitess-cdc/src/test/java/com/vervetica/cdc/connectors/vitess/VitessTestBase.java Outdated Show resolved Hide resolved
...itess-cdc/src/test/java/com/vervetica/cdc/connectors/vitess/table/VitessConnectorITCase.java Show resolved Hide resolved
...connector-vitess-cdc/src/test/java/com/vervetica/cdc/connectors/vitess/VitessSourceTest.java Show resolved Hide resolved
@s-gelazevicius
Copy link
Contributor Author

@fsk119 thank you for review. I already addressed some of your comments, and will fix other issues during next week.

@s-gelazevicius s-gelazevicius force-pushed the feature/add-vitess-connector branch 3 times, most recently from 8287469 to 2681454 Compare March 25, 2022 07:30
@leonardBang leonardBang mentioned this pull request Mar 31, 2022
Closed
13 tasks
@s-gelazevicius s-gelazevicius force-pushed the feature/add-vitess-connector branch 2 times, most recently from 7792e31 to 5d6144e Compare May 24, 2022 11:19
@gintarasm gintarasm force-pushed the feature/add-vitess-connector branch from d84b0b6 to b34c9f4 Compare July 22, 2022 13:18
@leonardBang leonardBang added this to the V2.3.0 milestone Jul 28, 2022
s-gelazevicius and others added 15 commits April 24, 2023 17:07
PatrickRen
PatrickRen reviewed Jun 11, 2023
View reviewed changes
Copy link
Contributor

@PatrickRen PatrickRen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@s-gelazevicius Thanks for the new Vitess connector! The logic overall looks good to me. I left some minor comments about docs and pom.

And could you rebase the latest master and resolve conflicts? Thanks!

docs/content/connectors/vitess-cdc.md Show resolved Hide resolved
docs/content/about.md Show resolved Hide resolved
flink-sql-connector-vitess-cdc/pom.xml Show resolved Hide resolved
flink-sql-connector-vitess-cdc/pom.xml Show resolved Hide resolved
PatrickRen added a commit to PatrickRen/flink-cdc that referenced this pull request Jun 15, 2023
@s-gelazevicius @PatrickRen
[vitess] Add Vitess CDC connector (apache#456)
d3e6636 
Co-authored-by: Qingsheng Ren <renqschn@gmail.com>
PatrickRen pushed a commit to PatrickRen/flink-cdc that referenced this pull request Jun 15, 2023
@s-gelazevicius @PatrickRen
[vitess] Add Vitess CDC connector (apache#456)
ee6bbea
@PatrickRen
Copy link
Contributor

PatrickRen commented Jun 15, 2023 •
edited
Loading

Hey @s-gelazevicius, thanks a lot for the awesome work on this Vitess connector! As we are approaching the feature freeze of Flink CDC version 2.4.0, we are going to make some patches on your current work and merge your commits to master. We'll preserve all your changes and keep the commit under your name and email address 😄

I squashed all commits into one to keep the history of master branch clean. The CI is running in my own Azure pipeline. I'll merge the commit into master once the CI is green.

The only thing we are still missing for the new Vitess connector is the end-to-end test case. Could you add one under flink-cdc-e2e-tests in another PR when you are available? Thanks! I added E2E cases in order to make the release more stable.

@leonardBang
Copy link
Contributor

Hey @s-gelazevicius, thanks a lot for the awesome work on this Vitess connector! As we are approaching the feature freeze of Flink CDC version 2.4.0, we are going to make some patches on your current work and merge your commits to master. We'll preserve all your changes and keep the commit under your name and email address 😄

I squashed all commits into one to keep the history of master branch clean. The CI is running in my own Azure pipeline. I'll merge the commit into master once the CI is green.

The only thing we are still missing for the new Vitess connector is the end-to-end test case. Could you add one under flink-cdc-e2e-tests in another PR when you are available? Thanks!

Thanks @s-gelazevicius and @PatrickRen for the contribution,
Could we keep both gintarasm and s-gelazevicius 's contributions when squash the commits? and I think that we should add an e2e test for new joined connector.

PatrickRen pushed a commit to PatrickRen/flink-cdc that referenced this pull request Jun 16, 2023
@s-gelazevicius @PatrickRen
[vitess] Add Vitess CDC connector (apache#456)
a88a27e
PatrickRen pushed a commit to PatrickRen/flink-cdc that referenced this pull request Jun 19, 2023
@s-gelazevicius @gintarasm @PatrickRen
[vitess] Add Vitess CDC connector (apache#456)
6943618 
Co-authored-by: gintarasm <gintaras.matulas@gmail.com>
@PatrickRen PatrickRen mentioned this pull request Jun 19, 2023
Merged
PatrickRen pushed a commit that referenced this pull request Jun 19, 2023
@s-gelazevicius @gintarasm @PatrickRen
[vitess] Add Vitess CDC connector (#456)
e349cb1 
Co-authored-by: gintarasm <gintaras.matulas@gmail.com>
@PatrickRen
Copy link
Contributor

Closed by commit e349cb1

@PatrickRen PatrickRen closed this Jun 19, 2023
@s-gelazevicius s-gelazevicius deleted the feature/add-vitess-connector branch August 23, 2023 08:17
@s-gelazevicius s-gelazevicius restored the feature/add-vitess-connector branch August 23, 2023 08:17
@s-gelazevicius s-gelazevicius deleted the feature/add-vitess-connector branch August 23, 2023 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PatrickRen PatrickRen PatrickRen left review comments

@leonardBang leonardBang Awaiting requested review from leonardBang

@fsk119 fsk119 Awaiting requested review from fsk119

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
V2.4.0
Development

Successfully merging this pull request may close these issues.
6 participants
@s-gelazevicius @leonardBang @MartijnVisser @PatrickRen @fsk119 @gintarasm