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][hotfix] Wait for consumer reset before job close #4578

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

yunfengzhou-hub
Copy link
Contributor

Purpose

Linked issue: #4442

This PR fixes the bug that Consumers might not have been reset yet when the streaming job is closed in test cases. This is because the resetting logic happens in MonitorFunction#notifyCheckpointComplete, and Flink does not guarantee the invocation of this method when the job is canceled or stopped.

This bug happens rarely in the current test cases because Flink's legacy source function runs independently in a separated thread. When we use FLIP-27 source for MonitorFunction and checkpoint events may pile up in Flink's Mailbox, this bug will be triggered more frequently.

Tests

Existing test cases are used to verify this change.

API and Format

This change does not affect API or storage format.

Documentation

This change does not affect introduce a new feature.

@wwj6591812
Copy link
Contributor

Hi, @yunfengzhou-hub .
You means we will use FLIP-27 source to rewrite MonitorFunction?
If yes, I have not see this change in https://github.com/apache/paimon/pull/4567/files.

@yunfengzhou-hub
Copy link
Contributor Author

Hi, @yunfengzhou-hub . You means we will use FLIP-27 source to rewrite MonitorFunction? If yes, I have not see this change in https://github.com/apache/paimon/pull/4567/files.

Hi @wwj6591812 . Yes we will rewrite it. In the PR you mentioned, MonitorFunction is renamed to MonitorSource, as it is not a SourceFunction any more.

Copy link
Contributor

@JingsongLi JingsongLi left a comment

Choose a reason for hiding this comment

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

+1

@JingsongLi JingsongLi merged commit 63551bb into apache:master Nov 25, 2024
12 checks passed
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.

3 participants