-
Notifications
You must be signed in to change notification settings - Fork 499
Description
Hi, thank you for this project.
I been testing this project in kubernetes with 100/200 Req/sec and with invalid status code (> 299) there are some problems with the memory and a lot of OOMKilled in a day(10-15).
I tested this PR updated with the current version of the branch main(#235) but it has other errors. (Probably channels closed)
error copying response: readfrom tcp 127.0.0.1:8080->127.0.0.1:53150: write tcp 127.0.0.1:8080->127.0.0.1:53150: write: broken pipe
This image is from prometheus with the current branch main.
Memory incrase until OOMKilled happens
I been trying some code modifications and this behavior changes when added this If (after this if https://github.com/willnorris/imageproxy/blob/main/imageproxy.go#L484-L487)
if should304(req, resp) {
// bare 304 response, full response will be used from cache
return &http.Response{StatusCode: http.StatusNotModified}, nil
}
if resp.StatusCode > 299 {
return nil, fmt.Errorf("invalid http code %s: %s", req.URL.String(), resp.Status)
}
and this. https://github.com/willnorris/imageproxy/blob/main/imageproxy.go#L207
i moved it before this if.
resp, err := p.Client.Do(actualReq)
// close the original resp.Body, even if we wrap it in a NopCloser below
if resp != nil && resp.Body != nil {
defer resp.Body.Close()
}
if err != nil {
msg := fmt.Sprintf("error fetching remote image: %v", err)
p.log(msg)
http.Error(w, msg, http.StatusInternalServerError)
metricRemoteErrors.Inc()
return
}
I'm no sure if this status code validation works in all escenarios, but for me it works.
This is prometheus with this change(1 hour before vs 9 hour now)
The memory problem is still present, but the behavior is different.
I added "net/http/pprof" to try to find aditional strange behaviors.
and I found this.
It seems like a problem with cached request too.
I found this open PR https://github.com/gregjones/httpcache/pull/102/files
This is from pprof:
Heap: https://drive.google.com/file/d/1lH9oYfO_88EAAMdxEwZV1FrcNS-3CaN-/view?usp=sharing
goroutine: https://drive.google.com/file/d/1_jinUorbPVNLxEMrUbzXs5Z9ITT5_2MD/view?usp=sharing