Batch stabilization #4647

michaelabuckley · 2023-03-14T22:39:17Z

Rework our batch2 framework for correctness.

Move all conditional JobInstance updates into write-locked transactions
Change all WorkChunk writes to use event transactions defined in IWorkChunkPersistence
Simplify stats calc
Clarify error handling during work chunk processing.

Avoid fetching work-chunk data (#4622)

* add end time to reduction step * add changelog --------- Co-authored-by: Long Ma <long@smilecdr.com> (cherry picked from commit 37f5e59)

Provide error message in cancelled jobs, and avoid transitions in final states.

codecov · 2023-03-21T03:18:09Z

Codecov Report

Patch coverage has no change and project coverage change: +0.11 🎉

Comparison is base (2f5ffe7) 81.32% compared to head (8c398c9) 81.43%.

❗ Current head 8c398c9 differs from pull request most recent head 41646c1. Consider uploading reports for the commit 41646c1 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #4647      +/-   ##
============================================
+ Coverage     81.32%   81.43%   +0.11%     
- Complexity    23650    24869    +1219     
============================================
  Files          1425     1525     +100     
  Lines         86399    91237    +4838     
  Branches      11677    12243     +566     
============================================
+ Hits          70265    74302    +4037     
- Misses        10947    11531     +584     
- Partials       5187     5404     +217

see 1475 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

…nstance-transactions # Conflicts: # hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/batch2/JpaJobPersistenceImpl.java # hapi-fhir-storage-batch2-test-utilities/src/main/java/ca/uhn/hapi/fhir/batch2/test/AbstractIJobPersistenceSpecificationTest.java # hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/api/IWorkChunkPersistence.java # hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/coordinator/JobCoordinatorImpl.java # hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/coordinator/WorkChannelMessageHandler.java # hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/model/WorkChunkCreateEvent.java # hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/coordinator/JobCoordinatorImplTest.java

# Conflicts: # hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/ExpandResourcesStep.java

Other cleanup

hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/Batch2JobHelper.java

hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/api/IWorkChunkPersistence.java

...-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/coordinator/JobDefinitionRegistry.java

hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/model/WorkChunkCreateEvent.java

…pa/test/Batch2JobHelper.java Co-authored-by: StevenXLi <stevenli_8118@hotmail.com>

fil512

Reviewed up to just before ReductionStepDataSink.accept()

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/batch2/JpaJobPersistenceImpl.java

...fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IBatch2JobInstanceRepository.java

...-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/coordinator/JobDefinitionRegistry.java

hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/coordinator/JobStepExecutor.java

* Use java event names for work chunk transitions. * Cherry-pick d5ebd1f from rel_6_4 Avoid fetching work-chunk data (#4622) * add end time to reduction step (#4640) * add end time to reduction step * add changelog --------- Co-authored-by: Long Ma <long@smilecdr.com> (cherry picked from commit 37f5e59) * Cancel processing Provide error message in cancelled jobs, and avoid transitions in final states. * Apply tx boundary to starting job and first chunk. * cleanup * Apply tx boundary to work chunk processing * Delete BatchWorkChunk * Introduce events for job create, and chunk dequeue * Apply tx boundary to chunk handler * Move instance cancellation to database * tx boundary around stats collection and completion * tx boundary around stats collection and completion * Extend tx boundary to error, fail, and cancel * Move failure into status calc * ERROR is not an "ended" state. * Revert generics cleanup to avoid noise * Avoid sending gated chunks twice. * Make no-data path safer. Cleanup * Fix mock test for step advance. * Delete unsafe updateInstace() call * Cleanup * Changelog and notes * Fix cancel boundary. Cleanups * Cleanup * Sort mongo chunks for stable paging. Other cleanup * Document error handling * Cleanup * Update hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/Batch2JobHelper.java Co-authored-by: StevenXLi <stevenli_8118@hotmail.com> --------- Co-authored-by: longma1 <32119004+longma1@users.noreply.github.com> Co-authored-by: StevenXLi <stevenli_8118@hotmail.com>

...-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/coordinator/ReductionStepDataSink.java

...ge-batch2/src/main/java/ca/uhn/fhir/batch2/coordinator/ReductionStepExecutorServiceImpl.java

hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/api/IJobPersistence.java

...-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/progress/JobInstanceStatusUpdater.java

...orage-batch2/src/test/java/ca/uhn/fhir/batch2/maintenance/JobMaintenanceServiceImplTest.java

hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/model/JobInstanceTest.java

hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/model/StatusEnumTest.java

hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/RandomDataHelper.java

Post review batch2 cleanup. Review fixes and cleanup from #4647 Deleted methods in IJobPersistence unused in prod paths, and replaced their usage in tests. Lots of docs. Replace copy-on-write pattern in JobDefinitionRegistry with simple ConcurrentHashMap Fixed bad mappings from job ERRORED state to external APIs. People think ERRORED is a failed state. ¯\_(ツ)_/¯ Added some spec tests for chunk purging Deprecated ERRORED. For deleting in 6.8 Lots of plans for 6.8. Too risky for 6.6

Use java event names for work chunk transitions.

aa31980

michaelabuckley changed the title ~~Use java event names for work chunk transitions.~~ Batch stabilization Mar 17, 2023

michaelabuckley marked this pull request as draft March 17, 2023 15:48

michaelabuckley and others added 6 commits March 17, 2023 12:11

Merge branch 'master' into mb-job-instance-transactions

03f84df

Cherry-pick d5ebd1f from rel_6_4

d14e256

Avoid fetching work-chunk data (#4622)

add end time to reduction step (#4640)

b22b7e1

* add end time to reduction step * add changelog --------- Co-authored-by: Long Ma <long@smilecdr.com> (cherry picked from commit 37f5e59)

Cancel processing

ab01e61

Provide error message in cancelled jobs, and avoid transitions in final states.

Apply tx boundary to starting job and first chunk.

2921d34

cleanup

6f9519c

michaelabuckley added 8 commits March 23, 2023 17:38

Apply tx boundary to work chunk processing

faf726a

Delete BatchWorkChunk

bdde3e5

Introduce events for job create, and chunk dequeue

3ab0266

Apply tx boundary to chunk handler

4e187c2

Move instance cancellation to database

67de89c

tx boundary around stats collection and completion

850ea86

tx boundary around stats collection and completion

92ac183

michaelabuckley mentioned this pull request Apr 6, 2023

Prototype batch maintenance in the db. #4550

Closed

michaelabuckley added 11 commits April 6, 2023 18:54

Extend tx boundary to error, fail, and cancel

b6d852d

Move failure into status calc

16d39a0

ERROR is not an "ended" state.

4fcaee4

Merge branch 'master' into mb-job-instance-transactions

b1954f6

# Conflicts: # hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/ExpandResourcesStep.java

Revert generics cleanup to avoid noise

5631fdb

Avoid sending gated chunks twice.

042e2fb

Make no-data path safer. Cleanup

4317caa

Fix mock test for step advance.

a61fff3

Delete unsafe updateInstace() call

8f73d86

Cleanup

9b55023

Changelog and notes

41ea816

michaelabuckley added 4 commits April 17, 2023 18:50

Fix cancel boundary. Cleanups

2932e93

Cleanup

e26a474

Sort mongo chunks for stable paging.

a2114c6

Other cleanup

Document error handling

4b1840b

michaelabuckley marked this pull request as ready for review April 18, 2023 16:57

michaelabuckley added 2 commits April 18, 2023 16:12

Merge branch 'master' into mb-job-instance-transactions

8b5fe4d

Cleanup

37e51f1

StevenXLi reviewed Apr 25, 2023

View reviewed changes

Update hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/j…

41646c1

…pa/test/Batch2JobHelper.java Co-authored-by: StevenXLi <stevenli_8118@hotmail.com>

jamesagnew approved these changes Apr 25, 2023

View reviewed changes

jamesagnew enabled auto-merge (squash) April 25, 2023 21:54

michaelabuckley disabled auto-merge April 25, 2023 22:01

tadgh merged commit 8813d9b into master Apr 25, 2023

tadgh deleted the mb-job-instance-transactions branch April 25, 2023 22:47

fil512 reviewed Apr 25, 2023

View reviewed changes

michaelabuckley mentioned this pull request Apr 26, 2023

Batch2 cleanup #4779

Merged

fil512 approved these changes Apr 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Batch stabilization #4647

Batch stabilization #4647

michaelabuckley commented Mar 14, 2023 •

edited

Loading

codecov bot commented Mar 21, 2023 •

edited

Loading

fil512 left a comment

Batch stabilization #4647

Batch stabilization #4647

Conversation

michaelabuckley commented Mar 14, 2023 • edited Loading

codecov bot commented Mar 21, 2023 • edited Loading

Codecov Report

fil512 left a comment

Choose a reason for hiding this comment

michaelabuckley commented Mar 14, 2023 •

edited

Loading

codecov bot commented Mar 21, 2023 •

edited

Loading