Skip to content

fix: park a migration whose chain task is killed - #337

Open
mihir-kandoi wants to merge 1 commit into
developfrom
fix/strand-migration-on-killed-task
Open

fix: park a migration whose chain task is killed#337
mihir-kandoi wants to merge 1 commit into
developfrom
fix/strand-migration-on-killed-task

Conversation

@mihir-kandoi

Copy link
Copy Markdown
Contributor

A killed chain task never reaches the except blocks that move a migration to needs_attention, so the operation froze mid-flight: the UI spun on "Updating" forever, Retry/Restore stayed hidden (both gated on needs_attention), and store.current() blocked every future update with MigrationConflictError. Recovery meant hand-editing the operation JSON.

Hit on a real bench — task 20260731-173422-815719 took a SIGTERM partway through update_apps and exited -15.

Each working state now declares the state it parks in, and MigrationOperation.strand() moves it there from outside the task process. Two callers:

  • a strand-migration callback declared by every chain link, fired by the task wrapper on a kill and by TaskProcess.reconcile() for an orphaned link
  • MigrationStore.get(), when the last chain task is terminal but the record still claims work is in progress — covers a reboot or a killed wrapper, where the callback never runs either

The six chain tasks were identical apart from the call they wrapped, so they now share MigrationChainTask.

A migration only ever advanced its own record from inside the task
process, in the except blocks of update_apps, back_up_site, migrate_site
and their revert counterparts. SIGTERM terminates CPython without
raising, so a killed chain link left the operation frozen in a working
state: the UI spun on "Updating" forever, retry/restore stayed hidden
because both are gated on needs_attention, and store.current() kept
blocking every future update with MigrationConflictError. Recovering
meant hand-editing the operation JSON.

Give each working state the failure state it parks in, and add
MigrationOperation.strand() to move there from outside the task
process. Two things call it:

- A strand-migration callback declared by every chain link, so the task
  wrapper fires it on a kill and TaskProcess.reconcile() fires it for a
  link that was orphaned. It is a no-op once the operation already
  reported the failure itself, which is the common case.
- MigrationStore.get(), when the last chain task is terminal but the
  record still says work is in progress. This covers the runs where the
  callback never got to fire either - a host reboot, or the wrapper
  itself being killed - so the operation heals on the next read.

The six chain tasks were identical apart from the call they wrapped, so
they now share MigrationChainTask, which owns the run/queue-next body
and declares the callback once.
@mihir-kandoi

Copy link
Copy Markdown
Contributor Author

@tanmoysrt not very sure about this PR, close if incorrect

@tanmoysrt

tanmoysrt commented Jul 31, 2026

Copy link
Copy Markdown
Member

It needs to fix at task layer ig. For OOM kill, need to catch that somehow to take better decision.
Then, we can chain the migrations.

Will push the other changes in this same PR.

@tanmoysrt

Copy link
Copy Markdown
Member

@beagle-app review

@beagle-app beagle-app left a comment

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.

IGNORE

@frappe frappe deleted a comment from beagle-app Aug 2, 2026
@frappe frappe deleted a comment from beagle-app Aug 2, 2026
@frappe frappe deleted a comment from beagle-app Aug 2, 2026
@frappe frappe deleted a comment from beagle-app Aug 2, 2026
@frappe frappe deleted a comment from beagle-app Aug 2, 2026
@frappe frappe deleted a comment from beagle-app Aug 2, 2026
@frappe frappe deleted a comment from beagle-app Aug 2, 2026
@frappe frappe deleted a comment from beagle-app Aug 2, 2026
@frappe frappe deleted a comment from beagle-app Aug 2, 2026
@tanmoysrt

Copy link
Copy Markdown
Member

This should be fixed. If any task job get killed, reconcillation process will make the task as failed after 1 minutes (to avoid race between starting job and assuming none is running it anymore).

Also, the jobs starts in seperate child, so parent kill shouldn't kill the jobs. Fixed some issue. Check, if it works fine on macos.

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.

3 participants