[
https://issues.apache.org/jira/browse/YUNIKORN-3370?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dale Richardson updated YUNIKORN-3370:
--------------------------------------
Description:
Follow-up to YUNIKORN-3357. Supersedes YUNIKORN-3366 (closed as a duplicate of
this issue) and replaces the original description of this issue, which claimed
the core services could not be restarted in-process. That claim was wrong:
{{EventSystemImpl.Stop()}} / {{StartServiceWithPublisher()}} have been
CAS-gated since YUNIKORN-3336, {{UserGroupCache.Stop()}} resets its singleton,
and core's own {{pkg/scheduler/tests}} mock scheduler already starts and stops
the full service set per test in one binary. The A/B in the shim (adding
{{coreContext.StopAll()}} to {{{}MockScheduler.stop(){}}}) that motivated the
original description no longer reproduces on master (0/15 failures either way).
What is actually wrong is test hygiene: tests in both repos start core services
and never stop them, so their goroutines outlive the test binary and the goleak
check has to exempt them.
Core (#1124), 5 exemptions:
* {{events.(*EventSystemImpl).StartServiceWithPublisher.func1}} and
{{{}events.(*eventPublisher).start.func1{}}}: tests call {{StartService()}}
without {{Stop()}}
* {{scheduler.(*partitionManager).cleanRoot}} and {{{}cleanExpiredApps{}}}:
tests build a {{ClusterContext}} / partition without {{Stop()}}
* {{{}common/security.(*UserGroupCache).run{}}}: tests resolve users without
stopping the cache
Shim (#1061), 17 exemptions, all core-service goroutines:
{{MockScheduler.stop()}} stops the shim but not the core it started. Adding
{{fc.coreContext.StopAll()}} lets all 17 be deleted; verified on the #1061
branch with the exemption list emptied.
Repro: delete the exemption in {{pkg/common/leakcheck/leakcheck.go}} and run
the package; goleak's {{VerifyTestMain}} fails the binary with the leaked stack.
Fix: {{defer <service>.Stop()}} / {{StopAll()}} in the affected tests, then
delete the exemptions. Can be split per package (events / partition cleaners /
UserGroupCache / shim mock).
was:
Follow-up to YUNIKORN-3357 (surfaced by shim PR #1061). Blocks burning down the
17 inherited core-service leakcheck exemptions in the shim.
yunikorn-core's service lifecycle is one-way: {{EventSystemImpl.Stop()}} niles
its channel and early-returns on a {{stopped}} flag that is never cleared, and
{{StartServiceWithPublisher}} starts its handler unconditionally, so a start
after a stop leaks a handler; process-global config callbacks and
{{UserGroupCache}} are likewise torn down and reused. Because of this the
shim's {{MockScheduler.stop()}} cannot call {{coreContext.StopAll()}} to clean
up the in-process core without introducing intermittent test failures.
Evidence: adding {{coreContext.StopAll()}} to {{MockScheduler.stop()}} produced
a ~25% {{TestAssumePodError}} flake at {{-count>1}} (3/12 and 3/10, versus 0/35
without it).
Proposed fix:
* deregister the callback in Stop() (or re-register in
StartServiceWithPublisher)
* use a unique id (uuid) instead of a unix-seconds timestamp
* delete the events exemptions in pkg/common/leakcheck/leakcheck.go in core
(#1124) and shim (#1061)
> yunikorn-core services are not restartable in-process, so the shim cannot
> stop the core it starts without intermittent failures
> -------------------------------------------------------------------------------------------------------------------------------
>
> Key: YUNIKORN-3370
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3370
> Project: Apache YuniKorn
> Issue Type: Test
> Components: shim - kubernetes
> Reporter: Dale Richardson
> Assignee: weichen lai
> Priority: Minor
> Labels: newbie
>
> Follow-up to YUNIKORN-3357. Supersedes YUNIKORN-3366 (closed as a duplicate
> of this issue) and replaces the original description of this issue, which
> claimed the core services could not be restarted in-process. That claim was
> wrong: {{EventSystemImpl.Stop()}} / {{StartServiceWithPublisher()}} have been
> CAS-gated since YUNIKORN-3336, {{UserGroupCache.Stop()}} resets its
> singleton, and core's own {{pkg/scheduler/tests}} mock scheduler already
> starts and stops the full service set per test in one binary. The A/B in the
> shim (adding {{coreContext.StopAll()}} to {{{}MockScheduler.stop(){}}}) that
> motivated the original description no longer reproduces on master (0/15
> failures either way).
> What is actually wrong is test hygiene: tests in both repos start core
> services and never stop them, so their goroutines outlive the test binary and
> the goleak check has to exempt them.
> Core (#1124), 5 exemptions:
> * {{events.(*EventSystemImpl).StartServiceWithPublisher.func1}} and
> {{{}events.(*eventPublisher).start.func1{}}}: tests call {{StartService()}}
> without {{Stop()}}
> * {{scheduler.(*partitionManager).cleanRoot}} and {{{}cleanExpiredApps{}}}:
> tests build a {{ClusterContext}} / partition without {{Stop()}}
> * {{{}common/security.(*UserGroupCache).run{}}}: tests resolve users without
> stopping the cache
> Shim (#1061), 17 exemptions, all core-service goroutines:
> {{MockScheduler.stop()}} stops the shim but not the core it started. Adding
> {{fc.coreContext.StopAll()}} lets all 17 be deleted; verified on the #1061
> branch with the exemption list emptied.
> Repro: delete the exemption in {{pkg/common/leakcheck/leakcheck.go}} and run
> the package; goleak's {{VerifyTestMain}} fails the binary with the leaked
> stack.
> Fix: {{defer <service>.Stop()}} / {{StopAll()}} in the affected tests, then
> delete the exemptions. Can be split per package (events / partition cleaners
> / UserGroupCache / shim mock).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
