whosonfirst / go-whosonfirst-spatial-www

Opinionated web application for the go-whosonfirst-spatial packages.
BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

HTTP response error when hammering the server #23

Open tomtaylor opened 1 year ago

tomtaylor commented 1 year ago

I'm sometimes seeing the following response from the client when making up to 16 PIP request at once, filling the CPU of a 16 core machine:

2022/08/12 09:56:06 http: superfluous response.WriteHeader call from github.com/whosonfirst/go-whosonfirst-spatial-pip/api.PointInPolygonHandler.func1 (http.go:126)

I don't think this is a client bug - I've disabled keep alives and am closing the connection after every request.

thisisaaronland commented 1 year ago

Line 126 in go-whosonfirst-spatial-pip/blob/main/api/http.go is returning an error marshaling the response JSON to the handler's response writer:

https://github.com/whosonfirst/go-whosonfirst-spatial-pip/blob/main/api/http.go#L122-L128

What that suggests to me is that the JSON encoder has started writing data (to rsp) before encountering an error and triggering Go http code to also write HTTP header data. Based on other people's comments online this appears to have been introduced in Go 1.17:

https://cs.opensource.google/go/go/+/refs/tags/go1.17.1:src/net/http/server.go;l=120-126

One option would be to replace the code to create a json.Encoder(io.Writer) with calls to json.Marshal(pip_rsp) and write the encoded data in to memory and then, errors notwithstanding, writing that data to the response handler. (Fun fact: you can tweak that code directly in vendor/github/whosonfirst... and rebuild things with go -mod vendor...)

In the case of PIP responses this probably isn't a big deal performance wise but fundamentally this feels like a weird decision in Go-land.

Are you able to see whether there are error responses from the api/pip endpoing that correspond with the log messages?