Skip to content

[FLINK-37979] Remove obsolete MySQL CDC snapshot split assigner code #4048

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

morozov
Copy link
Contributor

@morozov morozov commented Jun 21, 2025

There is a redundant condition and a misleading comment in MySqlSnapshotSplitAssigner#captureNewlyAddedTables() that complicate understanding of the code:

if (AssignerStatus.isAssigningFinished(assignerStatus)) {
// start the newly added tables process under binlog reading phase
LOG.info(
"Found newly added tables, start capture newly added tables process under binlog reading phase");
this.startAssignNewlyAddedTables();
}

By the time when the above condition is being evaluated, an identical condition has been already evaluated earlier in the code:

if (sourceConfig.isScanNewlyAddedTableEnabled()
&& !sourceConfig.getStartupOptions().isSnapshotOnly()
&& AssignerStatus.isAssigningFinished(assignerStatus)) {

Therefore, the second evaluation will always result in true and thus is redundant.

Code Change History

Prior to #3519, table discovery would happen in any assigner status, so the condition in the end of the method made sense. Together with adding this condition in the beginning of the method, #3519 should have removed it from the end.

What Exactly is Obsolete

  1. The comment:
    Since [FLINK-35859][cdc-connector] Fix: The assigner is not ready to offer finished split information, this should not be called #3519, if the job is still in the snapshot reading phase, this code won't be executed. This comment should be removed as misleading.
  2. The second if (AssignerStatus.isAssigningFinished(assignerStatus)) statement. While the captureNewlyAddedTables() method is running, the assigner status cannot be modified by a different thread because this code is only executed as part of MySqlSourceEnumerator#open(), i.e. the source in its initialization phase.

@morozov morozov marked this pull request as ready for review June 21, 2025 16:50
@morozov morozov changed the title FLINK-37979: Remove obsolete MySQL CDC snapshot split assigner code [FLINK-37979] Remove obsolete MySQL CDC snapshot split assigner code Jun 23, 2025
@morozov morozov force-pushed the FLINK-37979-obsolete-mysql-snapshot-split-assigner-code branch from d29ee2f to c4f6d26 Compare June 23, 2025 04:16
@leonardBang leonardBang requested a review from ruanhang1993 July 9, 2025 04:06
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.

1 participant