Dale Richardson created YUNIKORN-3357:
-----------------------------------------
Summary: 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
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 one act repo 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]
