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

Fix unfinished step in parallel flow #4567

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

Conversation

doontagi
Copy link

@doontagi doontagi commented Mar 24, 2024

related issue #3939

Motivation

Modification

  • Changed to wait for all asynchronous tasks to finish before throwing an exception.
  • Changed a job to handle exceptions after waiting for tasks currently being processed to complete. Prevent the job from terminating �even though there are remaining tasks.

Result

@doontagi doontagi force-pushed the fix-unfinished-step-in-parellel-flow branch from f101e58 to b8a1ba8 Compare March 24, 2024 14:23
}
}
}

if (!exceptions.isEmpty()) {
throw exceptions.get(0);
Copy link
Author

@doontagi doontagi Mar 27, 2024

Choose a reason for hiding this comment

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

As before, the first caught exception would be thrown.

jobBuilder.preventRestart().build().execute(execution);

boolean isExecutedNonExecutableStep = countDownLatch.await(1, TimeUnit.SECONDS);
assertEquals(BatchStatus.STOPPED, execution.getStatus());
Copy link
Author

@doontagi doontagi Mar 27, 2024

Choose a reason for hiding this comment

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

Finished with BatchStatus.STOPPED without executing nonExecutableStep.
This test would fail with executing nonExecutableStep if it runs on the current main branch.

@doontagi
Copy link
Author

Gentle ping to @fmbenhassine. Could you review this PR? I've fixed #3939.
Thanks!

@fmbenhassine
Copy link
Contributor

Thank you for your PR!

Changed to wait for all asynchronous tasks to finish before throwing an exception.

Isn't that not desired when one of the tasks is interrupted? Meaning if one wants to interrupt a step, the idea is not to wait for other steps to finish.

The change in this PR fixes the reported issue, I just want to make sure there is no side effect on the behaviour of step interruption.

@fmbenhassine fmbenhassine added the status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter label Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core pr-for: bug status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Different behavior when interrupting a job, depending on the parallel flow order
2 participants