[
https://issues.apache.org/jira/browse/YUNIKORN-3410?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dale Richardson updated YUNIKORN-3410:
--------------------------------------
Description:
A configuration reload that removes a partition, and the RM partition-removal
event ({{removePartitionsByRMID}}), both stop the partition manager while
holding the {{ClusterContext}} write lock. {{partitionManager.Stop()}} calls
{{remove()}} synchronously, and {{remove()}} ends with
{{cc.removePartition()}}, which takes that same lock. The lock is not
reentrant, so the infrastructure event goroutine blocks on itself and never
returns.
{code}func (manager *partitionManager) Stop() {
[...]
manager.remove() // synchronous since YUNIKORN-2233
}
func (manager *partitionManager) remove() {
[...]
manager.cc.removePartition(manager.pc.Name) // cc.Lock() again; the
caller already holds it
}
{code}
Once this happens everything that needs the cluster context lock stops: the
scheduling loop, every REST endpoint including health, and the RM callback
path. Reproduced during the July concurrency review with a test that removes a
partition through the config update path (not in the tree). The reachable
trigger is a hot config refresh that removes or renames a partition; the RM
partition-removal event is the other locked caller. The registration handler
itself cannot reach it (it returns early when any partition exists).
{{TestStopPartitionManager}} calls {{Stop()}} without holding the lock, which
is why the suite never sees it.
YUNIKORN-2233 turned {{go manager.remove()}} into a direct call to fix the
shutdown ordering, which is what introduced this (v1.5.0). Related dead code:
{{markPartitionForRemoval}}, the only writer of the partition {{stateTime}},
has never had a production caller; its unlocked {{stateTime}} write is only
reachable through it, so the same change should either remove that function or
fix the write.
Fix: drop the {{removePartition}} call from {{remove()}} and let the two locked
callers delete the entry from {{cc.partitions}} themselves
({{removePartitionsByRMID}} already does). Do not reintroduce {{go
manager.remove()}}; that reopens the problem 2233 fixed.
Marker: the config-update handler, {{removePartitionsByRMID}},
{{UpdateRMSchedulerConfig}} and {{updateSchedulerConfig}} in {{context.go}},
plus {{handlePartitionEvent}} in {{partition.go}}, carry this JIRA; the fix
removes them.
was:
A configuration reload that removes a partition, and an RM re-registration,
both stop the partition manager while holding the {{ClusterContext}} write
lock. {{partitionManager.Stop()}} calls {{remove()}} synchronously, and
{{remove()}} ends with {{cc.removePartition()}}, which takes that same lock.
The lock is not reentrant, so the infrastructure event goroutine blocks on
itself and never returns.
{code}func (manager *partitionManager) Stop() {
[...]
manager.remove() // synchronous since YUNIKORN-2233
}
func (manager *partitionManager) remove() {
[...]
manager.cc.removePartition(manager.pc.Name) // cc.Lock() again; the
caller already holds it
}
{code}
Once this happens everything that needs the cluster context lock stops: the
scheduling loop, every REST endpoint including health, and the RM callback
path. Reproduced during the July concurrency review with a test that removes a
partition through the config update path (not in the tree). The reachable
trigger is a hot config refresh that removes or renames a partition; the RM
re-registration path exists but is hard to hit with the stock shim.
{{TestStopPartitionManager}} calls {{Stop()}} without holding the lock, which
is why the suite never sees it.
YUNIKORN-2233 turned {{go manager.remove()}} into a direct call to fix the
shutdown ordering, which is what introduced this (v1.5.0). It also left
{{markPartitionForRemoval}} with no callers; that function's unlocked
{{stateTime}} write is only reachable through it, so the same change should
either remove the dead code or fix the write.
Fix: drop the {{removePartition}} call from {{remove()}} and let the two locked
callers delete the entry from {{cc.partitions}} themselves
({{removePartitionsByRMID}} already does). Do not reintroduce {{go
manager.remove()}}; that reopens the problem 2233 fixed.
Marker: the config-update and RM-registration handlers and
{{updateSchedulerConfig}} in {{context.go}}, plus {{handlePartitionEvent}} in
{{partition.go}}, carry this JIRA; the fix removes them.
> Partition removal from a config reload or RM re-registration self-deadlocks
> on the ClusterContext lock
> ------------------------------------------------------------------------------------------------------
>
> Key: YUNIKORN-3410
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3410
> Project: Apache YuniKorn
> Issue Type: Sub-task
> Components: core - scheduler
> Reporter: Dale Richardson
> Priority: Critical
>
> A configuration reload that removes a partition, and the RM partition-removal
> event ({{removePartitionsByRMID}}), both stop the partition manager while
> holding the {{ClusterContext}} write lock. {{partitionManager.Stop()}} calls
> {{remove()}} synchronously, and {{remove()}} ends with
> {{cc.removePartition()}}, which takes that same lock. The lock is not
> reentrant, so the infrastructure event goroutine blocks on itself and never
> returns.
> {code}func (manager *partitionManager) Stop() {
> [...]
> manager.remove() // synchronous since YUNIKORN-2233
> }
> func (manager *partitionManager) remove() {
> [...]
> manager.cc.removePartition(manager.pc.Name) // cc.Lock() again; the
> caller already holds it
> }
> {code}
> Once this happens everything that needs the cluster context lock stops: the
> scheduling loop, every REST endpoint including health, and the RM callback
> path. Reproduced during the July concurrency review with a test that removes
> a partition through the config update path (not in the tree). The reachable
> trigger is a hot config refresh that removes or renames a partition; the RM
> partition-removal event is the other locked caller. The registration handler
> itself cannot reach it (it returns early when any partition exists).
> {{TestStopPartitionManager}} calls {{Stop()}} without holding the lock, which
> is why the suite never sees it.
> YUNIKORN-2233 turned {{go manager.remove()}} into a direct call to fix the
> shutdown ordering, which is what introduced this (v1.5.0). Related dead code:
> {{markPartitionForRemoval}}, the only writer of the partition {{stateTime}},
> has never had a production caller; its unlocked {{stateTime}} write is only
> reachable through it, so the same change should either remove that function
> or fix the write.
> Fix: drop the {{removePartition}} call from {{remove()}} and let the two
> locked callers delete the entry from {{cc.partitions}} themselves
> ({{removePartitionsByRMID}} already does). Do not reintroduce {{go
> manager.remove()}}; that reopens the problem 2233 fixed.
> Marker: the config-update handler, {{removePartitionsByRMID}},
> {{UpdateRMSchedulerConfig}} and {{updateSchedulerConfig}} in {{context.go}},
> plus {{handlePartitionEvent}} in {{partition.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]
