fixup json,http protocols (#772)

* http now buffers the entire request body from the container before copying
it to the response writer (and sets content length). this is a level of sad i
don't feel comfortable talking about but it is what it is.
* json protocol was buffering the entire body so there wasn't any reason for
us to try to write this directly to the container stdin manually, we needed to
add a bufio.Writer around it anyway it was making too many write(fd) syscalls
with the way it was. this is just easier overall and has the same performance
as http now in my tests, whereas previously this was 50% slower [than http].
* add buffer pool for http & json to share/use. json doesn't create a new
buffer every stinkin request. we need to plumb down content length so that we
can properly size the buffer for json, have to add header size and everything
together but it's probably faster than malloc(); punting on properly sizing.
* json now sets content type to the length of the body from the returned json
blurb from the container

this does not handle imposing a maximum size of the response returned from a
container, which we need to add, but this has been open for some time
(specifically, on json). we can impose this by wrapping the pipes, but there's
some discussion to be had for json specifically we won't be able to just cut
off the output stream and use that (http we can do this). anyway, filing a
ticket...

closes #326 :(((((((
This commit is contained in:
Reed Allman
2018-02-14 14:06:36 -08:00
committed by GitHub
parent 9cbe4ea536
commit 04ae223a5d
2 changed files with 49 additions and 136 deletions

View File

@@ -2,15 +2,23 @@ package protocol
import (
"bufio"
"bytes"
"context"
"fmt"
"io"
"io/ioutil"
"net/http"
"strconv"
"sync"
"github.com/fnproject/fn/api/models"
opentracing "github.com/opentracing/opentracing-go"
)
var (
bufPool = &sync.Pool{New: func() interface{} { return new(bytes.Buffer) }}
)
// HTTPProtocol converts stdin/stdout streams into HTTP/1.1 compliant
// communication. It relies on Content-Length to know when to stop reading from
// containers stdout. It also mandates valid HTTP headers back and forth, thus
@@ -50,11 +58,20 @@ func (h *HTTPProtocol) Dispatch(ctx context.Context, ci CallInfo, w io.Writer) e
if err != nil {
return models.NewAPIError(http.StatusBadGateway, fmt.Errorf("invalid http response from function err: %v", err))
}
defer resp.Body.Close()
span, _ = opentracing.StartSpanFromContext(ctx, "dispatch_http_write_response")
defer span.Finish()
buf := bufPool.Get().(*bytes.Buffer)
buf.Reset()
defer bufPool.Put(buf)
// copy the response body into a buffer so that we read the whole thing. then set the content length.
io.Copy(buf, resp.Body)
resp.Body.Close()
resp.Body = ioutil.NopCloser(buf)
resp.Header.Set("Content-Length", strconv.Itoa(buf.Len()))
rw, ok := w.(http.ResponseWriter)
if !ok {
// async / [some] tests go through here. write a full http request to the writer