Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drain response body on exec call to enable keep-alive connection reuse #111

Merged
merged 2 commits into from
Feb 9, 2021

Conversation

Fiery-Fenix
Copy link
Contributor

@Fiery-Fenix Fiery-Fenix commented Feb 4, 2021

Currently library generates big amount or errors on ClickHouse side:
ServerErrorHandler: Poco::Exception. Code: 1000, e.code() = 104, e.displayText() = Connection reset by peer
That's happens because of improper close of response body.
Documentation in net/http/response code states that to reuse keep-alive clients require to both read to competition and close body:
https://go.googlesource.com/go/+/go1.15.7/src/net/http/response.go#63
Unfortunately library only close response body without reading response body which leads to dropping keep-alive connections instead of reuse.
Also good discussion about this issue could be found here: google/go-github#317
Tested on ClickHouse versions 19.16.12.49 and 20.8.12.2

@DoubleDi
Copy link
Collaborator

DoubleDi commented Feb 6, 2021

Hi! Awesome work!
Is it possible to close the Ping Body too?
This fixes #72

respBody, err := c.doRequest(ctx, req)

@Fiery-Fenix
Copy link
Contributor Author

Fiery-Fenix commented Feb 8, 2021

Hi! Awesome work!
Is it possible to close the Ping Body too?
This fixes #72

respBody, err := c.doRequest(ctx, req)

Nice catch! Added

@DoubleDi DoubleDi merged commit 828678e into mailru:master Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants