Hedger Lai created YUNIKORN-3446:
------------------------------------
Summary: [UGM] Race condition in ensureGroupTrackerForApp creates
duplicate GroupTrackers and bypasses group quota
Key: YUNIKORN-3446
URL: https://issues.apache.org/jira/browse/YUNIKORN-3446
Project: Apache YuniKorn
Issue Type: Bug
Components: core - scheduler
Reporter: Hedger Lai
Assignee: Hedger Lai
h3. Problem Description
In the User & Group Management (UGM) subsystem, when multiple applications
belonging to the same user and group are submitted concurrently, a race
condition in {{Manager.ensureGroupTrackerForApp}} causes multiple distinct
{{GroupTracker}} instances to be instantiated for the exact same group.
As a result:
1. Applications become bound to different, orphaned {{GroupTracker}} instances
in memory.
2. The global {{Manager}} only tracks the last overwritten {{GroupTracker}},
leaving the resource usage of earlier applications completely untracked at the
group level (*Silent Quota Bypass*).
3. When affected applications complete and call {{DecreaseTrackedResource}},
the canonical {{GroupTracker}} in the manager fails to find the application,
causing state and metric inconsistencies.
h3. Root Cause Analysis
In {{pkg/scheduler/ugm/manager.go}} (lines 243-252):
{code:go}
if appGroup != common.Empty {
groupTracker = m.GetGroupTracker(appGroup) // Step 1: Read lock check
if groupTracker == nil {
groupTracker = newGroupTracker(appGroup, m.events) // Step 2:
Unsynchronized allocation
m.Lock()
m.groupTrackers[appGroup] = groupTracker // Step 3: Blind overwrite
without double check!
m.Unlock()
}
}
userTracker.setGroupForApp(applicationID, groupTracker) // Step 4: Bound to app
{code}
This is a classic check-then-act / missing double-checked locking flaw:
* Multiple concurrent goroutines calling {{IncreaseTrackedResource}}
concurrently see {{groupTracker == nil}}.
* Goroutine 1 creates instance A and writes it into
{{m.groupTrackers[appGroup]}}.
* Goroutine 2 creates instance B and blindly overwrites
{{m.groupTrackers[appGroup]}} with instance B.
* Goroutine 1 binds instance A to its app, while Goroutine 2 binds instance B
to its app.
In contrast, {{getUserTracker}} in the same file (line 628) correctly
synchronizes and double-checks {{m.userTrackers}} under the write lock, but an
equivalent safe {{getGroupTracker}} helper was never implemented.
h3. Reproduce
We verified this locally with a concurrent chaos test launching 50 applications
under the same group simultaneously:
{code:text}
=== RUN TestEnsureGroupTrackerForAppConcurrentRace
manager_chaos_test.go:87: === CHAOS HARNESS TRIGGERED IN ROUND 2 ===
manager_chaos_test.go:88: Total applications launched: 50
manager_chaos_test.go:89: Distinct GroupTracker instances created in
memory: 3
manager_chaos_test.go:90: Applications linked to non-canonical orphaned
GroupTrackers: 3
manager_chaos_test.go:91: CRITICAL BUG DETECTED: Multiple GroupTracker
instances created for group 'devs'! Expected 1, found 3 instances (Round 2)
--- FAIL: TestEnsureGroupTrackerForAppConcurrentRace (0.00s)
{code}
h3. Proposed Fix
Implement a thread-safe private helper {{getGroupTracker(group string)
*GroupTracker}} in {{Manager}} that holds {{m.Lock()}} and checks existence
before allocating (mirroring {{getUserTracker}}), and invoke it inside
{{ensureGroupTrackerForApp}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
