[
https://issues.apache.org/jira/browse/YUNIKORN-3370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18112303#comment-18112303
]
Dale Richardson commented on YUNIKORN-3370:
-------------------------------------------
[~wilfreds] You are right on both counts, and I have re-checked the whole
description against master rather than just those two lines.
* {{{}stopped{}}}: the CAS pair in {{Stop()}} /
{{StartServiceWithPublisher()}} landed with YUNIKORN-3336 (#1111) on 29 July.
This JIRA was written against the pre-3336 code and filed two weeks after that
fix merged, so both statements about the event system were wrong when filed. A
start after a stop does not leak a handler.
* The callback: agreed, a registration is not a goroutine and {{reloadConfig}}
exits once the change is processed. Deregistering it would break turning the
event system on and off via config. Dropped.
I also re-ran the experiment the description rests on, on current k8shim master
({{{}edef7f7e{}}}, core {{{}2577453{}}}), adding {{coreContext.StopAll()}} to
{{{}MockScheduler.stop(){}}}:
* {{{}TestAssumePodError -count=15{}}}: 15/15 pass with and without the call
(it was ~25% failing when this was filed; the test has since been rewritten by
YUNIKORN-3355 and the retry loop by YUNIKORN-3369, and the original cause was
never pinned down).
* Full {{pkg/shim -count=2}} with the call: pass.
* With the call in place, all 17 core-service {{IgnoreTopFunction}} entries in
the shim's leakcheck (#1061) can be deleted and {{pkg/shim}} still passes under
goleak.
Core's own {{pkg/scheduler/tests}} mock scheduler already does
{{StartAllServices()}} / {{StopAll()}} per test inside one binary, which is the
in-process restart this JIRA said does not work.
So the "not restartable in-process" claim is withdrawn. What remains is what
YUNIKORN-3366 described before I closed it as a duplicate of this one: tests in
both repos start core services and never stop them. I have rewritten the
description accordingly; the fix is {{defer Stop()}} / {{StopAll()}} in the
tests, then deleting the exemptions in #1061 and #1124. If you would rather
keep this under the 3366 number, I can reopen that and close this instead
flagging since it changes the scope of what you picked up.
> 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: Bug
> Components: shim - kubernetes
> Reporter: Dale Richardson
> Assignee: weichen lai
> Priority: Major
> Labels: newbie
>
> 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)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
