fix: park a migration whose chain task is killed - #337
Open
mihir-kandoi wants to merge 1 commit into
Open
Conversation
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.
Contributor
Author
|
@tanmoysrt not very sure about this PR, close if incorrect |
Member
|
It needs to fix at task layer ig. For OOM kill, need to catch that somehow to take better decision. Will push the other changes in this same PR. |
Member
|
@beagle-app review |
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A killed chain task never reaches the
exceptblocks that move a migration toneeds_attention, so the operation froze mid-flight: the UI spun on "Updating" forever, Retry/Restore stayed hidden (both gated onneeds_attention), andstore.current()blocked every future update withMigrationConflictError. Recovery meant hand-editing the operation JSON.Hit on a real bench — task
20260731-173422-815719took a SIGTERM partway throughupdate_appsand 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:strand-migrationcallback declared by every chain link, fired by the task wrapper on a kill and byTaskProcess.reconcile()for an orphaned linkMigrationStore.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 eitherThe six chain tasks were identical apart from the call they wrapped, so they now share
MigrationChainTask.