wilfred-s commented on code in PR #533:
URL: https://github.com/apache/yunikorn-core/pull/533#discussion_r1407184297
##########
pkg/webservice/handlers.go:
##########
@@ -918,3 +918,61 @@ func getEvents(w http.ResponseWriter, r *http.Request) {
buildJSONErrorResponse(w, err.Error(),
http.StatusInternalServerError)
}
}
+
+func getStream(w http.ResponseWriter, r *http.Request) {
Review Comment:
We should limit the number of streams that we allow to be open at the same
time here. Even if it is some "high" number but we need to be able to turn down
new requests as a safety mechanism.
##########
pkg/webservice/webservice.go:
##########
@@ -63,12 +64,13 @@ func loggingHandler(inner http.Handler, name string)
http.HandlerFunc {
// TODO we need the port to be configurable
func (m *WebService) StartWebApp() {
router := newRouter()
+ // Important: do not use ReadTimeout, WriteTimeout or IdleTimeout
because those can break the event streaming
Review Comment:
This is a potential security issue. We need to be able to protect ourselves
from a Slowloris attack: YUNIKORN-2182
Since we are using go 1.20, or later, we should be looking at
https://github.com/golang/go/issues/54136 anyway to implement timeouts
##########
pkg/scheduler/objects/common_test.go:
##########
Review Comment:
not sure why these changes are needed.
##########
pkg/webservice/handlers.go:
##########
@@ -918,3 +918,61 @@ func getEvents(w http.ResponseWriter, r *http.Request) {
buildJSONErrorResponse(w, err.Error(),
http.StatusInternalServerError)
}
}
+
+func getStream(w http.ResponseWriter, r *http.Request) {
+ writeHeaders(w)
+ eventSystem := events.GetEventSystem()
+ if !eventSystem.IsEventTrackingEnabled() {
+ buildJSONErrorResponse(w, "Event tracking is disabled",
http.StatusInternalServerError)
+ return
+ }
+
+ f, ok := w.(http.Flusher)
+ if !ok {
+ buildJSONErrorResponse(w, "Writer does not implement
http.Flusher", http.StatusInternalServerError)
+ return
+ }
+
+ var count uint64
+ if countStr := r.URL.Query().Get("count"); countStr != "" {
+ var err error
+ count, err = strconv.ParseUint(countStr, 10, 64)
+ if err != nil {
+ buildJSONErrorResponse(w, err.Error(),
http.StatusBadRequest)
+ return
+ }
+ }
+
+ enc := json.NewEncoder(w)
+ stream := eventSystem.CreateEventStream(r.Host, count)
+
+ // Reading events in an infinite loop until either the client
disconnects or Yunikorn closes the channel.
+ // This results in a persistent HTTP connection where the message body
is never closed.
+ // We don't use timeouts and since HTTP 1.1 clients are expected to
handle persistent connections by default.
Review Comment:
See comment earlier, we should use the ResponseController with a sliding
write deadline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
