influxdata / influxdb1-client

The old clientv2 for InfluxDB 1.x
MIT License
190 stars 112 forks source link

Incomplete reading of body causes the connection not reusable #58

Closed ivankudibal closed 2 years ago

ivankudibal commented 2 years ago

Incomplete reading of body causes the connection not reusable

Forwarded from email:

Customer found a bug in our original Golang client which caused HTTP connections to not be reused. This means that every single query had to establish a new connection (so that’s time for TCP + HTTPS handshakes front-loaded onto every query).

Basically, the client hands the body of to the JSON decoder here: https://github.com/influxdata/influxdb1-client/blob/master/influxdb.go#L273

But, when the decoder reads the body it doesn’t read in the trailing newline (I guess because it’s found it’s final }). That leaves 1 byte unread from the body - because the body hasn’t been read in full, the connection can’t be re-used.

Suggested fix is to adjust the deferred close (https://github.com/influxdata/influxdb1-client/blob/master/influxdb.go#L273) to also perform a final read

        defer func() {
                io.Copy(ioutil.Discard, resp.Body)
                resp.Body.Close()
        }()

allowing the connection to be reused

influxdb.go

        dec := json.NewDecoder(resp.Body)
sranka commented 2 years ago

The client now closes the response body by calling defer resp.Body.Close() in both v1 and v2 versions. After the body arrives, Close() either

It is likely the case that new clients are created for every query and thus different connection pools are used.

sranka commented 2 years ago

Anyway, exhausting the response body will do no harm to the existing client and will force connection reuse when a query response fails while being larger than 256kB. See https://stackoverflow.com/questions/17948827/reusing-http-connections-in-go for details.