Skip to content

[3.0][cdc-runtime] add DataSinkWriterOperator to process Event#2649

Merged
ruanhang1993 merged 8 commits intoapache:masterfrom
lvyanquan0624:WriterOperator
Nov 9, 2023
Merged

[3.0][cdc-runtime] add DataSinkWriterOperator to process Event#2649
ruanhang1993 merged 8 commits intoapache:masterfrom
lvyanquan0624:WriterOperator

Conversation

@lvyanquan
Copy link
Copy Markdown
Contributor

@lvyanquan lvyanquan commented Nov 7, 2023

close #2605

points to be discussed:

  • should we add a new interface SupportSchemaEvolutionWriting, or let writer deal with it in write method?
  • Unnecessary.
  • is there a better way to access SinkWriter rather than reflection, as sinkWriter is private scope?
  • Reflection is acceptable.
  • SCHEMA_EVOLUTION_OPERATOR_ID is an identification to get SchemaOperatorCoordinator, does it meet the expectations of SchemaOperator?
  • OperatorID of SchemaOperator is created by Composer.

@PatrickRen @ruanhang1993 cc and Looking forward to your suggestions.

@lvyanquan lvyanquan changed the title [WIP][cdc-runtime] add DataSinkWriterOperator to process Event [WIP][3.0][cdc-runtime] add DataSinkWriterOperator to process Event Nov 7, 2023
@PatrickRen
Copy link
Copy Markdown
Contributor

PatrickRen commented Nov 7, 2023

@lvyanquan Thanks for the pull request!

  1. I don't quite get the usage of the interface SupportSchemaEvolutionWriting. Which component should this interface be applied to?
  2. Using reflections is acceptable to me. I couldn't find better solution for now. Otherwise we have to copy-paste and rewrite the entire SinkWriterOperator from Flink.
  3. I'm not sure if it is possible to build the operator ID in composing phase, instead of hard-coding in schema operator coordinator. It looks like the composer's responsibility to generate a ID and use it to construct the schema operator and the sink operator.

@lvyanquan
Copy link
Copy Markdown
Contributor Author

@PatrickRen

  1. I don't quite get the usage of the interface SupportSchemaEvolutionWriting. Which component should this interface be applied to?

SupportSchemaEvolutionWriting is designed for SinkWriter, just to separate the processing logic of SchemaChangeEvent and
DataChangeEvent. But it's somewhat redundant, and was deleted now.

  1. I'm not sure if it is possible to build the operator ID in composing phase, instead of hard-coding in schema operator coordinator. It looks like the composer's responsibility to generate a ID and use it to construct the schema operator and the sink operator.

Keep this code currently until another implementation is available

@lvyanquan lvyanquan changed the title [WIP][3.0][cdc-runtime] add DataSinkWriterOperator to process Event [3.0][cdc-runtime] add DataSinkWriterOperator to process Event Nov 8, 2023
Copy link
Copy Markdown
Contributor

@ruanhang1993 ruanhang1993 left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Copy Markdown
Contributor

@PatrickRen PatrickRen left a comment

Choose a reason for hiding this comment

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

@lvyanquan Thanks for the pull request! I left some comments.

Copy link
Copy Markdown
Contributor

@PatrickRen PatrickRen left a comment

Choose a reason for hiding this comment

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

Two tiny comments for the operator

Copy link
Copy Markdown
Contributor

@PatrickRen PatrickRen left a comment

Choose a reason for hiding this comment

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

@lvyanquan Sorry for flushing your thread! Found another issue.

Copy link
Copy Markdown
Contributor

@PatrickRen PatrickRen left a comment

Choose a reason for hiding this comment

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

@lvyanquan Thanks for the update! LGTM.

@ruanhang1993 ruanhang1993 merged commit 80dfc51 into apache:master Nov 9, 2023
@lvyanquan lvyanquan deleted the WriterOperator branch March 11, 2024 06:46
ChaomingZhangCN pushed a commit to ChaomingZhangCN/flink-cdc that referenced this pull request Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[flink-cdc-runtime] Add new SinkWriterOperator to process FlushEvent

3 participants