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

[BATCH-2760] Failed JobExecution due to unavailable TaskExecutor leaves End Time unpopulated #643

Closed
wants to merge 4 commits into from

Conversation

@dimitrisli
Copy link
Contributor

dimitrisli commented Sep 26, 2018

On instances when the taskExecutor is not picking up the submitted Job Execution, with this fix we are explicitly setting the End Time of the JobExecution in order not to give false positives via the JobExplorer#findRunningJobExecutions(). This last method is judging on running Job Executions based on End Time being null.

dimitrisli added 2 commits Sep 26, 2018
…es End Time unpopulated

On instances when the taskExecutor is not picking up the submitted Job Execution, with this fix we are explicitly setting the End Time of the JobExecution in order not to give false positives via the JobExplorer#findRunningJobExecutions(). This last method is judging on running Job Executions based on End Time being null.
[BATCH-2760] Failed JobExecution due to unavailable TaskExecutor leaves End Time unpopulated
@dimitrisli

This comment has been minimized.

Copy link
Contributor Author

dimitrisli commented Sep 26, 2018

I've used new Date(System.currentTimeMillis()) while setting the End Time, following the pattern used in JobExecution#lastUpdated() calls within the project as well as JobExecution#createTime field default.

@dimitrisli

This comment has been minimized.

Copy link
Contributor Author

dimitrisli commented Sep 27, 2018

dimitrisli added 2 commits Sep 29, 2018
…es End Time unpopulated

also adding setting the Start Time otherwise it's unpopulated as well. The Last Updated is getting set within the update() explicitly.
taskExecutor#execute() error
@dimitrisli

This comment has been minimized.

Copy link
Contributor Author

dimitrisli commented Sep 29, 2018

I've added:

  1. setting the Start Time as well as I figured it's also not getting set
  2. unit test for this fix mocking the TaskRejectedException getting thrown from the taskExecutor.
@mminella

This comment has been minimized.

Copy link
Member

mminella commented Oct 9, 2018

This PR implies that the job ran when in fact it didn't. Personally, I'd rather see us fix the logic in the places where we evaluate if a job is running to exclude those that haven't started in the first place. We can continue the discussion for this in the Jira issue BATCH-2675. I'm going to close this now until we determine a best course. We can always re-open it if we agree that this is the right path.

@mminella mminella closed this Oct 9, 2018
@dimitrisli

This comment has been minimized.

Copy link
Contributor Author

dimitrisli commented Oct 9, 2018

@mminella Thanks for the review. I'm continuing the discussion in BATCH-2675 with an alternative approach. thanks!

@dimitrisli

This comment has been minimized.

Copy link
Contributor Author

dimitrisli commented Oct 24, 2018

As a continuation of the above discussion in the comment section of: https://jira.spring.io/browse/BATCH-2675, I've raised a new PR: #659

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.