[YUNIKORN-1709] Add event streaming logic #533

pbacsko · 2023-04-28T11:08:51Z

What is this PR for?

Core implementation of event streaming. Clients are updated real time on the REST interface. A persistent HTTP connection is maintained for them.

Consumers can request a new stream and receive core scheduler events immediately on a read-only channel. Slow consumers are detected by checking the number of elements in the channel buffer. If the channel buffer on the local side is full, we remove the consumer.

Explicit closure of the event stream is possible and consumers are expected to do it.

What type of PR is it?

Todos

- Task

What is the Jira issue?

https://issues.apache.org/jira/browse/YUNIKORN-1709

How should this be tested?

Screenshots (if appropriate)

Questions:

- The licenses files need update.
- There is breaking changes for older versions.
- It needs documentation.

codecov · 2023-04-28T11:14:45Z

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (2291cea) 78.01% compared to head (a3b132c) 78.19%.

❗ Current head a3b132c differs from pull request most recent head 948b275. Consider uploading reports for the commit 948b275 to get more accurate results

Files	Patch %	Lines
pkg/events/event_system.go	28.57%	10 Missing ⚠️
pkg/webservice/handlers.go	89.85%	6 Missing and 1 partial ⚠️
pkg/events/event_streaming.go	97.53%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #533      +/-   ##
==========================================
+ Coverage   78.01%   78.19%   +0.17%     
==========================================
  Files          82       83       +1     
  Lines       13378    13559     +181     
==========================================
+ Hits        10437    10602     +165     
- Misses       2615     2630      +15     
- Partials      326      327       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/events/event_ringbuffer.go

pkg/events/event_streaming.go

pbacsko · 2023-11-22T18:20:08Z

Note: any timeout for the HTTP server can break the streaming (WriteTimeout for HTTP 1.1, ReadTimeout and IdleTimeout for HTTP 2.0). We either start a new server instance for the streaming or explicitly comment (or even test) that timeouts must not be used.

pkg/webservice/webservice.go

pkg/webservice/handlers.go

pkg/scheduler/objects/common_test.go

pkg/events/event_streaming.go

zhuqi-lucas

LGTM Left final minor comments!

pkg/events/event_streaming.go

pbacsko · 2023-12-08T14:50:07Z

ping @wilfred-s

wilfred-s

minor changes to comments on exported functions can be fixed during the commit also

pkg/events/event_system.go

pkg/events/event_ringbuffer.go

chia7712 · 2024-02-07T16:16:25Z

pkg/events/event_streaming.go

+	consumer chan *si.EventRecord,
+	stop chan struct{},
+	name string,
+	count uint64) {


this parameter is unused. Is it a potential bug or we can just remove it?

Need a follow up jira to clean that up @pbacsko

Correct. I actually have a list of necessary small code changes which are unrelated to each other, I'm adding this to the list.

pbacsko self-assigned this Apr 28, 2023

pbacsko marked this pull request as draft April 28, 2023 11:09

pbacsko force-pushed the YUNIKORN-1709 branch 4 times, most recently from 854fad6 to 94dedb4 Compare April 28, 2023 19:08

pbacsko requested a review from wilfred-s May 1, 2023 15:01

pbacsko marked this pull request as ready for review May 1, 2023 15:01

pbacsko commented May 1, 2023

View reviewed changes

pkg/events/event_ringbuffer.go Outdated Show resolved Hide resolved

pbacsko commented May 1, 2023

View reviewed changes

pkg/events/event_streaming.go Show resolved Hide resolved

pbacsko commented May 1, 2023

View reviewed changes

pkg/events/event_streaming.go Outdated Show resolved Hide resolved

pbacsko changed the title ~~[WIP] [YUNIKORN-1709] Store events separately for event streaming~~ [YUNIKORN-1709] Store events separately for event streaming May 1, 2023

pbacsko force-pushed the YUNIKORN-1709 branch 4 times, most recently from f750b87 to af2b4e0 Compare May 1, 2023 17:20

pbacsko changed the title ~~[YUNIKORN-1709] Store events separately for event streaming~~ [YUNIKORN-1709] Add event streaming logic May 1, 2023

pbacsko marked this pull request as draft May 30, 2023 11:24

pbacsko force-pushed the YUNIKORN-1709 branch from af2b4e0 to 899ee3e Compare August 7, 2023 13:53

pbacsko force-pushed the YUNIKORN-1709 branch 2 times, most recently from d95930a to 8633a35 Compare August 25, 2023 16:00

pbacsko force-pushed the YUNIKORN-1709 branch 5 times, most recently from a50058e to 535b6f5 Compare November 7, 2023 11:43

pbacsko marked this pull request as ready for review November 7, 2023 12:54

pbacsko requested review from manirajv06 and craigcondit November 7, 2023 12:54

pbacsko requested review from FrankYang0529 and zhuqi-lucas November 22, 2023 18:32

wilfred-s reviewed Nov 28, 2023

View reviewed changes

pkg/webservice/webservice.go Outdated Show resolved Hide resolved

pkg/webservice/handlers.go Show resolved Hide resolved

pkg/webservice/handlers.go Outdated Show resolved Hide resolved

pkg/scheduler/objects/common_test.go Outdated Show resolved Hide resolved

pbacsko force-pushed the YUNIKORN-1709 branch from 1cd63b8 to 090ba34 Compare November 29, 2023 11:46

zhuqi-lucas reviewed Nov 29, 2023

View reviewed changes

pkg/events/event_streaming.go Show resolved Hide resolved

zhuqi-lucas reviewed Nov 29, 2023

View reviewed changes

pkg/events/event_streaming.go Show resolved Hide resolved

pbacsko force-pushed the YUNIKORN-1709 branch 3 times, most recently from 431c6ee to b9e11cd Compare November 30, 2023 10:56

pbacsko added 8 commits November 30, 2023 21:34

[YUNIKORN-1709] Add event streaming logic

ebedaed

Unit test coverage, responses

681ca0a

Comments

dca4cd3

Tiny simplification

e26f17e

Comments, minor code change in getStream()

8509d46

Use http.NewResponseController()

e9b97da

test refactor

438a7df

change defaultChannelBufSize to 1000

a3b132c

pbacsko force-pushed the YUNIKORN-1709 branch from b9e11cd to a3b132c Compare November 30, 2023 20:36

zhuqi-lucas reviewed Dec 1, 2023

View reviewed changes

pkg/events/event_streaming.go Outdated Show resolved Hide resolved

zhuqi-lucas reviewed Dec 1, 2023

View reviewed changes

pkg/events/event_streaming.go Show resolved Hide resolved

zhuqi-lucas reviewed Dec 1, 2023

View reviewed changes

pkg/events/event_streaming.go Show resolved Hide resolved

zhuqi-lucas approved these changes Dec 1, 2023

View reviewed changes

code review changes

948b275

zhuqi-lucas reviewed Dec 1, 2023

View reviewed changes

pkg/events/event_streaming.go Show resolved Hide resolved

wilfred-s approved these changes Jan 24, 2024

View reviewed changes

pkg/events/event_system.go Show resolved Hide resolved

pkg/events/event_system.go Show resolved Hide resolved

pkg/events/event_ringbuffer.go Show resolved Hide resolved

pbacsko closed this in 681a4cb Jan 24, 2024

chia7712 reviewed Feb 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[YUNIKORN-1709] Add event streaming logic #533

[YUNIKORN-1709] Add event streaming logic #533

pbacsko commented Apr 28, 2023 •

edited

Loading

codecov bot commented Apr 28, 2023 •

edited

Loading

pbacsko commented Nov 22, 2023 •

edited

Loading

zhuqi-lucas left a comment

pbacsko commented Dec 8, 2023 •

edited

Loading

wilfred-s left a comment

chia7712 Feb 7, 2024

wilfred-s Feb 8, 2024

pbacsko Feb 8, 2024

[YUNIKORN-1709] Add event streaming logic #533

[YUNIKORN-1709] Add event streaming logic #533

Conversation

pbacsko commented Apr 28, 2023 • edited Loading

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

codecov bot commented Apr 28, 2023 • edited Loading

Codecov Report

pbacsko commented Nov 22, 2023 • edited Loading

zhuqi-lucas left a comment

Choose a reason for hiding this comment

pbacsko commented Dec 8, 2023 • edited Loading

wilfred-s left a comment

Choose a reason for hiding this comment

chia7712 Feb 7, 2024

Choose a reason for hiding this comment

wilfred-s Feb 8, 2024

Choose a reason for hiding this comment

pbacsko Feb 8, 2024

Choose a reason for hiding this comment

pbacsko commented Apr 28, 2023 •

edited

Loading

codecov bot commented Apr 28, 2023 •

edited

Loading

pbacsko commented Nov 22, 2023 •

edited

Loading

pbacsko commented Dec 8, 2023 •

edited

Loading