Skip to content

[FLINK-37386] Emit CreateTableEvent only when met the related SourceRecord. - #3932

Merged
leonardBang merged 9 commits into
apache:masterfrom
lvyanquan:FLINK-37386
Mar 12, 2025
Merged

[FLINK-37386] Emit CreateTableEvent only when met the related SourceRecord.#3932
leonardBang merged 9 commits into
apache:masterfrom
lvyanquan:FLINK-37386

Conversation

@lvyanquan

Copy link
Copy Markdown
Contributor

Emit CreateTableEvent only when met the related SourceRecord to avoid downstream backpressure.

@lvyanquan lvyanquan changed the title [FLINK-37278] Emit CreateTableEvent only when met the related SourceRecord. [FLINK-37386] Emit CreateTableEvent only when met the related SourceRecord. Feb 26, 2025
@lvyanquan

Copy link
Copy Markdown
Contributor Author

Hi @yuxiqian.

Could you help to review this?

} else {
if (isDataChangeRecord(element) || isSchemaChangeEvent(element)) {
TableId tableId = getTableId(element);
if (!alreadySendCreateTableTables.contains(tableId)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems alreadySendCreateTableTables and createTableEventCache are not being stored into MySQL source state persistently. Will that block users from recovering from an earlier binlog state where schemas are different from current state?

TableId tableId = getTableId(element);
if (!alreadySendCreateTableTables.contains(tableId)) {
CreateTableEvent createTableEvent = createTableEventCache.get(tableId);
// New created table in binlog reading phase.

@yuxiqian yuxiqian Feb 28, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

--- Create Table A --> Table A Binlog --> Drop Table A --> Startup Offset --->

                    ^
                    |
       What if we start from here?

Seems testDanglingDropTableEventInBinlog will fail.

@lvyanquan lvyanquan Feb 28, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is indeed a problem, but it is different from the one I hope to solve here.

For DataChange, perhaps we can derive the schema from SourceRecord instead of using SQL to query the latest schema. But to make this problem more difficult, if we start reading from a position where SchemaChange happened, we cannot derive the original schema.

--- Create Table A --> Alter Table A Add a column--> Table A Binlog --> Startup Offset --->

                     ^
                     |
        What if we start from here?

So I think starting from a position where historical schema is different with the current schema is still an unresolved issue for us.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for clarifying this. Should we throw an exception for such dangling schema change events here?

@lvyanquan

Copy link
Copy Markdown
Contributor Author

The issue is related to #3912 (comment) @gongzexin as you may be interested about it.

@leonardBang
leonardBang requested a review from yuxiqian March 5, 2025 11:43

@yuxiqian yuxiqian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for @lvyanquan's great work, just left some minor comments.

@yuxiqian yuxiqian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Would @leonardBang like to take a look?

@leonardBang leonardBang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

@leonardBang
leonardBang merged commit eee0cc0 into apache:master Mar 12, 2025
SML0127 pushed a commit to SML0127/flink-cdc-connectors that referenced this pull request Mar 12, 2025
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