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-37320] [Observer] FINISHED finite streaming jobs incorrectly being set to RECONCILING #944

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

luca-p-castelli
Copy link
Contributor

@luca-p-castelli luca-p-castelli commented Feb 18, 2025

What is the purpose of the change

This pull request fixes a bug where the observer tries changing the JOB STATUS of a FINISHED finite/bounded streaming job back to RECONCILING if the JM is MISSING.

Brief change log

Wraps AbstractFlinkDeploymentObserver.observeJmDeployment in an if-statement that only executes if the job is NOT in a terminal state.

Verifying this change

  • Tested locally
  • Unit test to guard against accidental future regression

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): No
  • The public API, i.e., is any changes to the CustomResourceDescriptors: No
  • Core observer or reconciler logic that is regularly executed: Yes

Documentation

  • Does this pull request introduce a new feature? No
  • If yes, how is the feature documented? not applicable

@luca-p-castelli luca-p-castelli marked this pull request as ready for review February 18, 2025 17:07
deploymentStatus.getJobStatus().setState(JobStatus.RECONCILING);

if (!ReconciliationUtils.isJobInTerminalState(deploymentStatus)) {
deploymentStatus.getJobStatus().setState(JobStatus.RECONCILING);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add a unit test to guard this change against accidental future regressions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All done. Happy to make any other changes.

@gyfora
Copy link
Contributor

gyfora commented Feb 20, 2025

rebase on main for docker fix

@gyfora gyfora merged commit f874b03 into apache:main Feb 21, 2025
115 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.

2 participants