[
https://issues.apache.org/jira/browse/YUNIKORN-3436?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dale Richardson updated YUNIKORN-3436:
--------------------------------------
Description:
Sibling of YUNIKORN-3364, which covers the history-replay send at stream open.
The forwarder goroutine started by {{EventStreaming.CreateEventStream}} has a
second bare blocking send inside its main loop
({{pkg/events/event_streaming.go}}, the {{consumer <- event}} after the
{{seen}} check, currently line 127). It sits inside the {{for}} but outside the
{{select}}, so once the forwarder is parked on it neither {{stop}} nor
{{e.stopCh}} can reach it.
Trigger: a streaming client that has fallen at least 1000 events behind (the
{{consumer}} buffer is full, so the forwarder is parked on the bare send) and
then disconnects or stops reading. The handler's deferred {{RemoveStream}}
closes the stream's stop channel, but the forwarder cannot observe it, and
nothing drains {{consumer}} again. The same happens when {{PublishEvent}}
evicts the stream for a full {{local}} buffer ("Listener buffer full due to
potentially slow consumer, removing it"). A client that disconnects while its
buffer is not full does not leak: the forwarder reaches the {{select}}, and the
closed stop channel wins within a few iterations.
Exposure: the endpoint has no in-tree consumer; it serves external tools (event
exporters, usage trackers), so this needs a persistently slow external client
on a cluster busy enough to build a 1000-event backlog. Each such episode pins
one forwarder goroutine, its two 1000-entry buffers and the {{seen}} map for
the life of the process. The stream is removed from {{eventStreams}} and the
host is released from the streaming limiter, so leaked forwarders are neither
visible through {{GetEventStreams}} nor counted against {{maxStreams}}; a
reconnecting slow client can accumulate them without bound.
{{TestEventStreaming_SlowConsumer}} reproduces it: it creates a stream,
publishes 2500 events with no reader, asserts the stream was evicted, and
{{Close()}} cannot stop the forwarder. This is the one
{{CreateEventStream.func1}} goroutine that still leaks under goleak after the
test-hygiene fix in YUNIKORN-3372 landed (#1135), and it is not addressed by
the fix for YUNIKORN-3364 in #1133, which makes only the history-replay send
selectable.
Proposed fix: the same shape as YUNIKORN-3364, applied to the in-loop send:
{code:go}select {
case consumer <- event:
case <-stop:
close(consumer)
return
case <-e.stopCh:
close(consumer)
return
}
{code}
Once both sends are selectable, {{TestEventStreaming_SlowConsumer}} passes
under goleak without an exemption, and the
{{events.(*EventStreaming).CreateEventStream}} entry in
{{pkg/common/leakcheck/leakcheck.go}} (#1124) can be deleted.
Found with goleak while rebasing #1124 (YUNIKORN-3357).
was:
Sibling of YUNIKORN-3364, which covers the history-replay send at stream open.
The forwarder goroutine started by {{EventStreaming.CreateEventStream}} has a
second bare blocking send inside its main loop
({{pkg/events/event_streaming.go}}, the {{consumer <- event}} after the
{{seen}} check, currently line 127). It sits inside the {{for}} but outside the
{{select}}, so once the forwarder is parked on it neither {{stop}} nor
{{e.stopCh}} can reach it.
This is reachable in production through the slow-consumer eviction path:
# A REST event-stream client stops reading; the handler stalls in {{Encode}}
under its 5s write deadline.
# The forwarder keeps draining {{local}} into {{consumer}} until {{consumer}}
(buffer 1000) is full, then blocks on the bare send.
# {{PublishEvent}} finds {{local}} full, logs "Listener buffer full due to
potentially slow consumer, removing it" and calls {{removeEventStream}}, which
closes the stream's stop channel. The forwarder cannot observe it.
# The handler returns; its deferred {{RemoveStream}} is a no-op (already
removed). Nothing ever drains {{consumer}}.
Result: one forwarder goroutine plus its two 1000-entry buffers and {{seen}}
map are pinned for the life of the process, per slow-consumer episode. The
stream count exposed by {{GetEventStreams}} goes back to zero, so the leak is
invisible to the API.
{{TestEventStreaming_SlowConsumer}} reproduces it exactly: it creates a stream,
publishes 2500 events with no reader, asserts the stream was evicted, and
{{Close()}} cannot stop the forwarder. This is the one
{{CreateEventStream.func1}} goroutine that still leaks under goleak after the
test-hygiene fix in YUNIKORN-3372 landed (#1135), and it is not addressed by
the fix for YUNIKORN-3364 in #1133, which makes only the history-replay send
selectable.
Proposed fix: the same shape as YUNIKORN-3364, applied to the in-loop send:
{code:go}select {
case consumer <- event:
case <-stop:
close(consumer)
return
case <-e.stopCh:
close(consumer)
return
}
{code}
Once both sends are selectable, {{TestEventStreaming_SlowConsumer}} passes
under goleak without an exemption, and the
{{events.(*EventStreaming).CreateEventStream}} entry in
{{pkg/common/leakcheck/leakcheck.go}} (#1124) can be deleted.
Found with goleak while rebasing #1124 (YUNIKORN-3357).
Priority: Minor (was: Major)
> Event stream forwarder leaks on slow-consumer eviction: in-loop consumer send
> is not selectable
> -----------------------------------------------------------------------------------------------
>
> Key: YUNIKORN-3436
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3436
> Project: Apache YuniKorn
> Issue Type: Bug
> Components: core - scheduler
> Reporter: Dale Richardson
> Assignee: weichen lai
> Priority: Minor
>
> Sibling of YUNIKORN-3364, which covers the history-replay send at stream
> open. The forwarder goroutine started by {{EventStreaming.CreateEventStream}}
> has a second bare blocking send inside its main loop
> ({{pkg/events/event_streaming.go}}, the {{consumer <- event}} after the
> {{seen}} check, currently line 127). It sits inside the {{for}} but outside
> the {{select}}, so once the forwarder is parked on it neither {{stop}} nor
> {{e.stopCh}} can reach it.
> Trigger: a streaming client that has fallen at least 1000 events behind (the
> {{consumer}} buffer is full, so the forwarder is parked on the bare send) and
> then disconnects or stops reading. The handler's deferred {{RemoveStream}}
> closes the stream's stop channel, but the forwarder cannot observe it, and
> nothing drains {{consumer}} again. The same happens when {{PublishEvent}}
> evicts the stream for a full {{local}} buffer ("Listener buffer full due to
> potentially slow consumer, removing it"). A client that disconnects while its
> buffer is not full does not leak: the forwarder reaches the {{select}}, and
> the closed stop channel wins within a few iterations.
> Exposure: the endpoint has no in-tree consumer; it serves external tools
> (event exporters, usage trackers), so this needs a persistently slow external
> client on a cluster busy enough to build a 1000-event backlog. Each such
> episode pins one forwarder goroutine, its two 1000-entry buffers and the
> {{seen}} map for the life of the process. The stream is removed from
> {{eventStreams}} and the host is released from the streaming limiter, so
> leaked forwarders are neither visible through {{GetEventStreams}} nor counted
> against {{maxStreams}}; a reconnecting slow client can accumulate them
> without bound.
> {{TestEventStreaming_SlowConsumer}} reproduces it: it creates a stream,
> publishes 2500 events with no reader, asserts the stream was evicted, and
> {{Close()}} cannot stop the forwarder. This is the one
> {{CreateEventStream.func1}} goroutine that still leaks under goleak after the
> test-hygiene fix in YUNIKORN-3372 landed (#1135), and it is not addressed by
> the fix for YUNIKORN-3364 in #1133, which makes only the history-replay send
> selectable.
> Proposed fix: the same shape as YUNIKORN-3364, applied to the in-loop send:
> {code:go}select {
> case consumer <- event:
> case <-stop:
> close(consumer)
> return
> case <-e.stopCh:
> close(consumer)
> return
> }
> {code}
> Once both sends are selectable, {{TestEventStreaming_SlowConsumer}} passes
> under goleak without an exemption, and the
> {{events.(*EventStreaming).CreateEventStream}} entry in
> {{pkg/common/leakcheck/leakcheck.go}} (#1124) can be deleted.
> Found with goleak while rebasing #1124 (YUNIKORN-3357).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
