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

[FLINK-36114] Make SchemaRegistryRequestHandler thread safe by blocking more schema change events #3563

Merged
merged 7 commits into from
Aug 22, 2024

Conversation

yuxiqian
Copy link
Contributor

@yuxiqian yuxiqian commented Aug 22, 2024

This closes FLINK-36114.

Currently, SchemaRegistry asynchronously receives schema change requests from SchemaOperator, and results of multiple requests might got mixed up together, causing incorrect logic flow in multiple parallelism cases.

Changing SchemaRegistry's behavior to accept requests in serial should resolve this problem.


This PR also changes MySQL pipeline source behavior.

Previously, some subTask might not emit CreateTableEvents if there are no corresponding SnapshotSplit assigned to them. This might cause problems if they are assigned some StreamSplits later, since their downstream Transform nodes never received CreateTableEvent and doesn't know about the schema.

By forcing MySQL source emitting CreateTableEvents when transiting from Snapshot to BinLog stage should resolve this problem.


This PR also adjusts Pipeline E2e test case parallelism to 1 to 4 to verify this change.

@leonardBang
Copy link
Contributor

Thanks @yuxiqian for the contribution, I left some comments @loserwang1024 Would you like to take a look at this PR whrn you have time?

Copy link
Contributor

@loserwang1024 loserwang1024 left a comment

Choose a reason for hiding this comment

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

I have left some comments.

TableId tableId = event.tableId();
Optional<Schema> latestSchema = getLatestOriginalSchema(tableId);
return Boolean.TRUE.equals(
SchemaChangeEventVisitor.visit(
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder we should do it in registry or sink? @leonardBang , WDYT?

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 @yuxiqian for the cooperation fix, LGTM, I believe your effort will make the schema evolution more stable.

@leonardBang leonardBang merged commit ee843e2 into apache:master Aug 22, 2024
20 checks passed
yuxiqian added a commit to yuxiqian/flink-cdc that referenced this pull request Aug 22, 2024
…afe by blocking subsequent schemaChangeEvent

This closes apache#3563.

Co-authored-by: Hongshun Wang <loserwang1024@gmail.com>

(cherry picked from commit ee843e2)
leonardBang pushed a commit to yuxiqian/flink-cdc that referenced this pull request Aug 27, 2024
…afe by blocking subsequent schemaChangeEvent

This closes apache#3563.

Co-authored-by: Hongshun Wang <loserwang1024@gmail.com>

(cherry picked from commit ee843e2)
leonardBang pushed a commit that referenced this pull request Aug 27, 2024
…afe by blocking subsequent schemaChangeEvent

This closes #3563.

Co-authored-by: Hongshun Wang <loserwang1024@gmail.com>

(cherry picked from commit ee843e2)
qiaozongmi pushed a commit to qiaozongmi/flink-cdc that referenced this pull request Sep 23, 2024
…afe by blocking subsequent schemaChangeEvent

This closes apache#3563.

Co-authored-by: Hongshun Wang <loserwang1024@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants