@marcel91200 / caddy Mirror · read-only / commits / 881b826f
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 14, 05:15 UTC

reverseproxy: Pool copy buffers (minor optimization)

Matthew Holt committed May 27, 2020 at 11:42 UTC 881b826fb59a25102fd14ae3b420639479f2d6bf
2 files changed +20 -48
modules/caddyhttp/reverseproxy/reverseproxy.go
+7 -33
@@ -587,20 +587,15 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, di Dia
587 rw.WriteHeader(res.StatusCode)
588
589 err = h.copyResponse(rw, res.Body, h.flushInterval(req, res))
590 + res.Body.Close() // close now, instead of defer, to populate res.Trailer
591 if err != nil {
591 - defer res.Body.Close()
592 - // Since we're streaming the response, if we run into an error all we can do
593 - // is abort the request. Issue golang/go#23643: ReverseProxy should use ErrAbortHandler
594 - // on read error while copying body.
595 - // TODO: Look into whether we want to panic at all in our case...
596 - if !shouldPanicOnCopyError(req) {
597 - // p.logf("suppressing panic for copyResponse error in test; copy error: %v", err)
598 - return err
599 - }
600 -
601 - panic(http.ErrAbortHandler)
592 + // we're streaming the response and we've already written headers, so
593 + // there's nothing an error handler can do to recover at this point;
594 + // the standard lib's proxy panics at this point, but we'll just log
595 + // the error and abort the stream here
596 + h.logger.Error("aborting with incomplete response", zap.Error(err))
597 + return nil
598 }
603 - res.Body.Close() // close now, instead of defer, to populate res.Trailer
599
600 if len(res.Trailer) > 0 {
601 // Force chunking if we saw a response trailer.
@@ -679,27 +674,6 @@ func (h Handler) directRequest(req *http.Request, di DialInfo) {
674 req.URL.Host = reqHost
675 }
676
682 -// shouldPanicOnCopyError reports whether the reverse proxy should
683 -// panic with http.ErrAbortHandler. This is the right thing to do by
684 -// default, but Go 1.10 and earlier did not, so existing unit tests
685 -// weren't expecting panics. Only panic in our own tests, or when
686 -// running under the HTTP server.
687 -// TODO: I don't know if we want this at all...
688 -func shouldPanicOnCopyError(req *http.Request) bool {
689 - // if inOurTests {
690 - // // Our tests know to handle this panic.
691 - // return true
692 - // }
693 - if req.Context().Value(http.ServerContextKey) != nil {
694 - // We seem to be running under an HTTP server, so
695 - // it'll recover the panic.
696 - return true
697 - }
698 - // Otherwise act like Go 1.10 and earlier to not break
699 - // existing tests.
700 - return false
701 -}
702 -
677 func copyHeader(dst, src http.Header) {
678 for k, vv := range src {
679 for _, v := range vv {
modules/caddyhttp/reverseproxy/streaming.go
+13 -15
@@ -13,8 +13,8 @@
13 // limitations under the License.
14
15 // Most of the code in this file was initially borrowed from the Go
16 -// standard library, which has this copyright notice:
17 -// Copyright 2011 The Go Authors.
16 +// standard library and modified; It had this copyright notice:
17 +// Copyright 2011 The Go Authors
18
19 package reverseproxy
20
@@ -24,6 +24,8 @@ import (
24 "net/http"
25 "sync"
26 "time"
27 +
28 + "go.uber.org/zap"
29 )
30
31 func (h Handler) handleUpgradeResponse(rw http.ResponseWriter, req *http.Request, res *http.Response) {
@@ -97,19 +99,8 @@ func (h Handler) copyResponse(dst io.Writer, src io.Reader, flushInterval time.D
99 }
100 }
101
100 - // TODO: Figure out how we want to do this... using custom buffer pool type seems unnecessary
101 - // or maybe it is, depending on how we want to handle errors,
102 - // see: https://github.com/golang/go/issues/21814
103 - // buf := bufPool.Get().(*bytes.Buffer)
104 - // buf.Reset()
105 - // defer bufPool.Put(buf)
106 - // _, err := io.CopyBuffer(dst, src, )
107 - var buf []byte
108 - // if h.BufferPool != nil {
109 - // buf = h.BufferPool.Get()
110 - // defer h.BufferPool.Put(buf)
111 - // }
112 - // // we could also see about a pool that returns values like this: make([]byte, 32*1024)
102 + buf := streamingBufPool.Get().([]byte)
103 + defer streamingBufPool.Put(buf)
104 _, err := h.copyBuffer(dst, src, buf)
105 return err
106 }
@@ -131,6 +122,7 @@ func (h Handler) copyBuffer(dst io.Writer, src io.Reader, buf []byte) (int64, er
122 // something we need to report to the client, but read errors are a problem on our
123 // end for sure. so we need to decide what we want.)
124 // p.logf("copyBuffer: ReverseProxy read error during body copy: %v", rerr)
125 + h.logger.Error("reading from backend", zap.Error(rerr))
126 }
127 if nr > 0 {
128 nw, werr := dst.Write(buf[:nr])
@@ -221,3 +213,9 @@ func (c switchProtocolCopier) copyToBackend(errc chan<- error) {
213 _, err := io.Copy(c.backend, c.user)
214 errc <- err
215 }
216 +
217 +var streamingBufPool = sync.Pool{
218 + New: func() interface{} {
219 + return make([]byte, 32*1024)
220 + },
221 +}