[
https://issues.apache.org/jira/browse/YUNIKORN-3404?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated YUNIKORN-3404:
-------------------------------------
Labels: pull-request-available (was: )
> PlaceholderManager Start and Stop operations have race conditions
> -----------------------------------------------------------------
>
> Key: YUNIKORN-3404
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3404
> Project: Apache YuniKorn
> Issue Type: Bug
> Components: shim - kubernetes
> Reporter: Andy Huang
> Assignee: Andy Huang
> Priority: Major
> Labels: pull-request-available
>
> The Kubernetes shim PlaceholderManager lifecycle implementation has race
> conditions and shutdown gaps around its running state and stop channel.
> PlaceholderManager.Start() currently checks the running atomic and updates it
> in separate operations:
> {code:go}
> if mgr.isRunning() {
> return
> }
> mgr.setRunning(true)
> go func() {
> // ...
> }()
> {code}
> Concurrent Start() calls can both observe running=false before either call
> stores true, starting multiple cleanup goroutines for the same
> PlaceholderManager.
> PlaceholderManager.Stop() has a similar check-then-act pattern:
> {code:go}
> if !mgr.isRunning() {
> return
> }
> mgr.stopChan <- struct{}{}
> {code}
> Concurrent Stop() calls can both observe running=true. Only one send can be
> received before the cleanup goroutine exits, potentially leaving another
> caller blocked indefinitely on the unbuffered stop channel.
> There is an additional shutdown problem. The cleanup goroutine alternates
> between receiving from stopChan and cleanOrphanPlaceholders(). The latter
> holds the manager lock and can issue Delete calls without a deadline. If
> shutdown occurs while an orphan sweep is blocked on a slow API server, Stop()
> blocks on its unbuffered send until the complete sweep returns.
> The running state is changed asynchronously by the cleanup goroutine after
> receiving the stop signal. Stop() therefore does not guarantee that the
> manager has stopped when it returns, and Start() and Stop() can leave
> lifecycle state inconsistent when invoked concurrently.
> The lifecycle operations should be atomic, idempotent, and shutdown-safe. A
> likely approach is to use CompareAndSwap or sync.Once for lifecycle
> transitions, close a stop channel exactly once instead of sending on it, and
> provide a reliable way to wait for the cleanup goroutine to exit.
> Expected behavior:
> * At most one PlaceholderManager cleanup goroutine can run.
> * Concurrent Start() calls do not start duplicate goroutines.
> * Stop() is safe and idempotent.
> * Concurrent Stop() calls do not block or panic.
> * Shutdown does not wait indefinitely for an orphan cleanup blocked on an
> API call.
> * When Stop() returns, the manager has either fully stopped or exposes a
> reliable way to wait for shutdown completion.
> * Start() and Stop() do not leave the running state inconsistent.
> This was identified while reviewing KubernetesShim shutdown fixes:
> * https://issues.apache.org/jira/browse/YUNIKORN-3367
> * [https://github.com/apache/yunikorn-k8shim/pull/1074]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
