search

fractal-analytics-platform / fractal-tasks-core

Main tasks for the Fractal analytics platform
https://fractal-analytics-platform.github.io/fractal-tasks-core/
BSD 3-Clause "New" or "Revised" License
14 stars 6 forks source link

Fix for race condition in apply_registration_to_image #517

Closed jluethi closed 12 months ago

jluethi commented 12 months ago

Addresses part of https://github.com/fractal-analytics-platform/fractal-tasks-core/issues/516 by making it less likely to happen.

I'm creating a PR to make this race condition way less likely:

Instead of removing the old group first (slow call), then renaming the new group to the old name, I'll now do the following: Rename the old group to group_name_tmp (e.g. (0_tmp). Then rename the new group to the old group name (e.g. from 0_registered to 0). Both those renamings are very fast. Only after that, remove the tmp folder. The opening of the old zarr group gets a try except to catch the zarr.errors.GroupNotFoundError and would try again 5 seconds later. This should make it very unlikely to hit the issue. It's not closing the issue yet, see description there.

Checklist before merging

github-actions[bot] commented 12 months ago

Coverage report

The coverage rate went from 89.91% to 89.84% :arrow_down: The branch rate is 83%.

75% of new lines are covered.

Diff Coverage details (click to unfold) ### fractal_tasks_core/tasks/apply_registration_to_image.py `75%` of new lines are covered (`79.35%` of the complete file). Missing lines: `206`, `222`, `223`, `224`
jluethi commented 12 months ago

These tasks are not tested in example 03 in fractal-demos and it seems to run well. In 5 runs, I could not reproduce this race condition anymore.

See: https://github.com/fractal-analytics-platform/fractal-demos/tree/main/examples/03_cardio_multiplexing