[
https://issues.apache.org/jira/browse/YUNIKORN-3357?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dale Richardson reassigned YUNIKORN-3357:
-----------------------------------------
Assignee: Dale Richardson
> Add goroutine leak detection to core tests via uber-go/goleak
> -------------------------------------------------------------
>
> Key: YUNIKORN-3357
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3357
> Project: Apache YuniKorn
> Issue Type: Improvement
> Components: core - common, core - scheduler
> Reporter: Dale Richardson
> Assignee: Dale Richardson
> Priority: Major
> Labels: pull-request-available
>
> Yunikorn tests currently leak goroutines silently: components started by a
> test (event system, partition manager cleaners, user group cache) outlive the
> test binary with nobody noticing, and one intermittent shutdown race makes a
> leaked goroutine appear in ~43% of full pkg/scheduler/tests runs.
> Proposal: wire [uber-go/goleak|https://github.com/uber-go/goleak] into every
> test package in core/k8shim repos via a shared {{pkg/common/leakcheck}}
> helper.
> * One {{TestMain}} per test package
> * All exemptions live in a single (per repo) documented list in
> {{{}leakcheck.options(){}}}; {{VerifyTestMain}} accepts per-package extra
> options so the shared list never needs widening
> * No existing test or production code is modified; goleak is already a
> transitive dependency and becomes a direct test-only one ({{{}go.sum{}}}
> unchanged)
> Switching detection on in Yunikorn-core surfaced seven pre-existing
> leaked-goroutine shapes. Each gets a documented baseline exemption so this
> change lands green and blocks new kinds of leaks immediately; each exemption
> is to be deleted by its own follow-up fix:
> || # ||Goroutine||Cause||
> |1|{{EventSystemImpl.StartServiceWithPublisher.func1}}|tests start the event
> system without {{Stop()}}|
> |2|{{eventPublisher.start.func1}}|{{Init()}} registers a configmap callback
> that {{Stop()}} never removes — a config update after shutdown restarts the
> event system|
> |3|{{EventStreaming.CreateEventStream.func1}}|(a) test never calls
> {{{}RemoveStream{}}}; (b) forwarder blocks on a bare {{consumer <- event}}
> send, so slow-consumer eviction cannot release it|
> |4|{{partitionManager.cleanRoot}}|tests build a {{ClusterContext}} without
> {{Stop()}}|
> |5|{{partitionManager.cleanExpiredApps}}|same as 4|
> |6|{{security.UserGroupCache.run}}|same as 4 — {{ClusterContext.Stop()}}
> already stops it|
> |7|{{ClusterContext.notifyRMNewAllocation}}|shutdown race: {{StopAll}} stops
> the scheduler before the RM proxy; a queued allocation event can win the
> select over the closed stop channel, then the notify blocks forever on its
> unbuffered reply because {{handleRMEvents}} exits without draining|
> Items 2, 3(b) and 7 are production shutdown gaps rather than test hygiene and
> will be filed as separate JIRAs (linked).
> Known limitation, stated in the code: exemptions match on top stack frame, so
> the baseline is a ratchet against new leak kinds, not an instance count;
> three entries key on positional {{.funcN}} closure names that the follow-up
> fixes should replace by hoisting the goroutine bodies to named methods.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
