Skip to content

Execution Engine jobs are not invalidated automatically (#1268)#1272

Merged
pavgra merged 6 commits into
rc-2.7.3from
issue-1268-add-invalidate-scheduler
Aug 6, 2019
Merged

Execution Engine jobs are not invalidated automatically (#1268)#1272
pavgra merged 6 commits into
rc-2.7.3from
issue-1268-add-invalidate-scheduler

Conversation

@YaroslavTir

Copy link
Copy Markdown

Fixes #1268

  • add scheduler class ExecutionEngineAnalysisStatusInvalidationScheduler, that is active only in case execution.invalidation.maxage and execution.invalidation.period are set
  • small improvements

ymolodkov added 2 commits August 1, 2019 13:50
- add scheduler class ExecutionEngineAnalysisStatusInvalidationScheduler, that is active only in case `execution.invalidation.maxage` and `execution.invalidation.period` are set
- small improvements
- remove redundant Transaction annotation

@pavgra pavgra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When you click on FAILED status of invalidated PLP execution, popup doesn't arise. Should be
image

- invalidate job itself (only ExecutionEngineAnalysisStatus was invalidated until now)
@YaroslavTir

Copy link
Copy Markdown
Author

When you click on FAILED status of invalidated PLP execution, popup doesn't arise. Should be
image

indeed, Only status was invalidated, but not a job. I fixed it with the last commit.

@pavgra pavgra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as before exit_message is empty. How have you tested it?

@pavgra pavgra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as before exit_message is empty. How have you tested it?

@pavgra

pavgra commented Aug 2, 2019

Copy link
Copy Markdown
Contributor
  1. First, scheduler calls invalidateExecutions -> jobInvalidator.invalidateJobExecutionById -> jobRepository.update
  2. exit_code = Failed together with exit_message = Invalidated by system are properly persisted in database
  3. Breakpoint in org.springframework.batch.core.job.AbstractJob line 341 (finally section of job execution) show exitStatus with empty message which is written to DB and overwrites previous proper data

@Component
public class JobInvalidator {

public static final String INVALIDATED_BY_SYSTEM_EXIT_CODE = "Invalidated by system";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this a 'code' or a message?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Message. So I agree, would be good to rename the variable

- fix: empty exitMessage for failed task.
Comment thread pom.xml Outdated
Comment thread src/main/java/org/ohdsi/webapi/JobInvalidator.java Outdated
ymolodkov added 2 commits August 5, 2019 16:55
- reuse code from JobInvalidator
- set execution.invalidation.maxage as 12 hours
- rename execution.invalidation.maxage -> execution.invalidation.maxage.hours
- remove unused updateAnalysisStatus method
@pavgra pavgra merged commit 9986644 into rc-2.7.3 Aug 6, 2019
@pavgra pavgra deleted the issue-1268-add-invalidate-scheduler branch August 6, 2019 12:26
anthonysena pushed a commit that referenced this pull request Aug 8, 2019
* Remove produces annotation + minor improvements (#1232)
* Fix overlap logic.
Allow event cohort periods that start within the target cohort era.  Prior version required the event cohort era to start and end in the target cohort era.
When collapsing, ensure start-end dates span at least a day.
Use count_big when counting cohort events and pathways.
Pad end dates to make non-inclusive.
Added casts for impala
Added temp-table cleanup.
Fixes #779.

* Omit cohort expression from list. - Fixes #219.
* Added client cache directives to response header (#1231)
* Removed legacy service which exposed server side request forgery issue (#1241)
* Added handlers for unexpected errors (#1237)
* Restored Kerberos auth support (#1253)
* Fixes masquerading of credentials in PLE / PLP results (#1255)
* Better quotes handling
* exclude older sql render versions (#1259)
* Fix compatability of /cohortdefinition/{id} endpoint (#1262)
* fixes cycling dependencies (#1264)
* Fix grouping problem in bq (#1267) (#1269)
* Fixes failed Cohort Pathways on Impala (#1274)
* fixes counts as bigint
* fixes Pathways for BQ
* Fixes save of Cohort Definitions when criteria requiring narrower CDM range are added (#1278)
* Fixes timeout based invalidation of Execution Engine jobs (#1272)
* Fixes Impala drivers (#1280)
* Updates circe to newer release which resolves PLE issues on Impala
* Merge fixes
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.

4 participants