Dale Richardson created YUNIKORN-3421:
-----------------------------------------
Summary: 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: Bug
Components: shim - kubernetes
Reporter: Dale Richardson
{{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]
