[
https://issues.apache.org/jira/browse/YUNIKORN-3421?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andy Huang reassigned YUNIKORN-3421:
------------------------------------
Assignee: (was: Andy Huang)
> shouldAppRelease drops the task lock inside an FSM callback: deadlock with a
> concurrent task event
> --------------------------------------------------------------------------------------------------
>
> Key: YUNIKORN-3421
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3421
> Project: Apache YuniKorn
> Issue Type: Sub-task
> Components: shim - kubernetes
> Reporter: Dale Richardson
> Priority: Critical
>
> {{Task.handle}} takes {{task.lock}} and holds it across the whole FSM event;
> {{looplab/fsm}} in turn holds its own event mutex across the
> before-callbacks. {{beforeTaskCompleted}}, {{beforeTaskFail}} and
> {{beforeTaskAllocated}} call {{releaseAllocation}}, which calls:
> {code}func (task *Task) shouldAppRelease() bool {
> task.lock.Unlock() // fsm's event mutex is still held
> defer task.lock.Lock()
> return task.application.tryAddReleasableTask(task)
> }
> {code}
> The lock is dropped because {{tryAddReleasableTask}} takes the application
> lock. While it is dropped a second goroutine can drive an event on the same
> task: the scheduling loop calls {{task.handle(InitTask)}} directly on New
> tasks, the dispatcher delivers {{CompleteTask}} for a pod deleted before it
> was scheduled. That goroutine takes the freed {{task.lock}} and blocks on the
> fsm mutex; the first returns and blocks re-taking {{task.lock}}. Neither has
> a timeout; recovery is a restart. A second cycle sits in the same window:
> {{releaseAllocation}} re-reads the state through {{GetTaskState()}} (a second
> fsm read lock on the same goroutine) while the core's RM callback can queue a
> writer in between via {{MarkPreviouslyAllocated}}, which wedges the RM
> callback goroutine as well.
> Both were reproduced end to end during the July concurrency review, driving
> nothing but real {{AddPod}}/{{DeletePod}} through the real dispatcher,
> scheduling loop and embedded core (the first after a few seconds of churn
> against one application, 5/5 at the shipped 1 s tick; the second on a
> recovery-shaped workload). Those tests are not in the tree; the fork fix
> branch carries deterministic ones that fail on master and pass with the fix.
> Once wedged the dispatcher and the scheduling loop are stuck, and in the
> second case so is all core-to-shim traffic. go-deadlock reports the first by
> timeout and cannot see the second at all.
> {{shouldAppRelease}} came in with YUNIKORN-3089 (deferring the release of
> tasks whose application the core has not accepted yet) and is in the released
> 1.9.0. {{flushReleaseableTasks}} on the application side only stays clear of
> the same self-deadlock because {{releaseAllocation(true)}} short-circuits
> before {{tryAddReleasableTask}}; any fix here must keep that, or restructure
> it away.
> Fix: run the release from after-callbacks, where fsm has released its locks,
> and pass the pre-transition state as {{event.Src}} instead of re-reading it;
> guard {{releaseableTasks}} with its own leaf mutex so
> {{tryAddReleasableTask}} no longer needs the application lock and
> {{shouldAppRelease}} no longer drops the task lock. A fix exists on the
> tigerquoll fork (PR #1 there) and will be filed once this has a number.
> Marker: {{shouldAppRelease}} in {{task.go}} and the {{flushReleaseableTasks}}
> banner in {{application.go}} carry this JIRA; the fix removes them.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
