-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[fix][cdc-base] Flink CDC base registers the identical history engine on multiple tasks #1340
Conversation
...ain/java/com/ververica/cdc/connectors/base/source/reader/external/JdbcSourceScanFetcher.java
Outdated
Show resolved
Hide resolved
...n/java/com/ververica/cdc/connectors/base/source/reader/external/JdbcSourceStreamFetcher.java
Outdated
Show resolved
Hide resolved
if (source.schema().fields().stream().anyMatch(r -> SCHEMA_NAME_KEY.equals(r.name()))) { | ||
return source.getString(SCHEMA_NAME_KEY); | ||
} | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to add a test for this method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think we only need changes in this class and add some tests for it. Other changes need to be removed from this PR.
...t/java/com/ververica/cdc/connectors/base/experimental/fetch/MySqlSourceFetchTaskContext.java
Outdated
Show resolved
Hide resolved
...ava/com/ververica/cdc/connectors/base/source/reader/external/JdbcSourceFetchTaskContext.java
Outdated
Show resolved
Hide resolved
...-base/src/test/java/com/ververica/cdc/connectors/base/MySqlChangeEventSourceExampleTest.java
Outdated
Show resolved
Hide resolved
Thanks @fuyun2024 for the contribution, could you help rebase this PR to latest master? |
9ba9235
to
cd610f4
Compare
93c656f
to
7281781
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @fuyun2024 for the contribution, thanks @molsionmo and @molsionmo for the review work, LGTM
issues : issues-1130