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

[mysql] Support tables which do not contain a primary key (#1827) #2150

Merged
merged 4 commits into from
May 30, 2023
Merged

[mysql] Support tables which do not contain a primary key (#1827) #2150

merged 4 commits into from
May 30, 2023

Conversation

ruanhang1993
Copy link
Contributor

This PR implements #1827.

@ruanhang1993 ruanhang1993 mentioned this pull request May 22, 2023
Closed
zstraw
zstraw reviewed May 26, 2023
View reviewed changes
Copy link

@zstraw zstraw 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 pushing this issue forward.

NIT: The doc has not updated.

About the question I mentioned before, shall we provide users with the option of exactly-once but worse performance if no pk table?

...ctor-mysql-cdc/src/main/java/com/ververica/cdc/connectors/mysql/source/utils/ChunkUtils.java Outdated Show resolved Hide resolved
...dc/src/main/java/com/ververica/cdc/connectors/mysql/source/assigners/MySqlChunkSplitter.java Outdated Show resolved Hide resolved
leonardBang
leonardBang reviewed May 29, 2023
View reviewed changes
Copy link
Contributor

@leonardBang leonardBang left a comment

Choose a reason for hiding this comment

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

Thanks @ruanhang1993 for the contribution, I left some comments.

...-cdc/src/main/java/com/ververica/cdc/connectors/mysql/debezium/reader/BinlogSplitReader.java Outdated Show resolved Hide resolved
...dc/src/main/java/com/ververica/cdc/connectors/mysql/debezium/reader/SnapshotSplitReader.java Outdated Show resolved Hide resolved
...or-mysql-cdc/src/main/java/com/ververica/cdc/connectors/mysql/source/MySqlSourceBuilder.java Outdated Show resolved Hide resolved
...tor-mysql-cdc/src/main/java/com/ververica/cdc/connectors/mysql/source/utils/RecordUtils.java Outdated Show resolved Hide resolved
...src/main/java/com/ververica/cdc/connectors/mysql/source/config/MySqlSourceConfigFactory.java Outdated Show resolved Hide resolved
@ruanhang1993
[mysql] Support tables which do not contain a primary key (apache#1827)
55c8809 
Co-authored-by: zhuyuan03 <zhuyuan03@corp.netease.com>
leonardBang
leonardBang approved these changes May 30, 2023
View reviewed changes
Copy link
Contributor

@leonardBang leonardBang left a comment

Choose a reason for hiding this comment

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

Thanks @ruanhang1993 for the update, LGTM

...tor-mysql-cdc/src/main/java/com/ververica/cdc/connectors/mysql/source/utils/RecordUtils.java
if (isDelete) {
if (records == null || records.isEmpty()) {
LOG.error(
"Deleting a record which is not in its split for tables without primary keys. This may happen when the chunk key column is updated in another snapshot split.");
Copy link
Contributor

Choose a reason for hiding this comment

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

we need open a doc PR to guide user how to choose chunk key column

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 will provide the docs about these experimental features later. See #2164.

@ruanhang1993 ruanhang1993 merged commit 0f8023d into apache:master May 30, 2023
14 checks passed
@ruanhang1993 ruanhang1993 mentioned this pull request May 30, 2023
Closed
@ruanhang1993
Copy link
Contributor Author

Thanks for pushing this issue forward.

NIT: The doc has not updated.

About the question I mentioned before, shall we provide users with the option of exactly-once but worse performance if no pk table?

Hi, @zstraw .
The single split is actually hard to use for users. All table data will be read into the memory and may cause OOM.
So this implementation requires the chunk key column when the table does not have primary keys.

I will provide the docs about these experimental features.

@ruanhang1993 ruanhang1993 deleted the nopk branch October 23, 2023 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zstraw zstraw zstraw left review comments

@leonardBang leonardBang leonardBang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ruanhang1993 @leonardBang @zstraw