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

Fix HTTP keep-alive for CouchDB connections #4497

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Conversation

nono
Copy link
Member

@nono nono commented Nov 28, 2024

Golang can reuse HTTP connections with keep alive, but the response body must have been fully read and closed. In the couchdb package, we were reading the body for sucessful response with a json.Decoder. It only reads a JSON value, and most responses from CouchDB have an additional \n after that JSON value. This last byte was not read from the response body, and it prevents Golang from reusing the connection. So, we are doing a new HTTP connection (with DNS resolution and TLS handshake) for each request to CouchDB.

Golang can reuse HTTP connections with keep alive, but the response body
must have been fully read and closed. In the couchdb package, we were
reading the body for sucessful response with a json.Decoder. It only
reads a JSON value, and most responses from CouchDB have an additional
\n after that JSON value. This last byte was not read from the response
body, and it prevents Golang from reusing the connection. So, we are
doing a new HTTP connection (with DNS resolution and TLS handshake) for
each request to CouchDB.
@nono nono requested review from sblaisot and paultranvan November 28, 2024 10:11
@nono nono requested a review from a team as a code owner November 28, 2024 10:11
Copy link
Contributor

@sblaisot sblaisot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, thx @nono

@nono nono merged commit 2cbb312 into master Nov 28, 2024
4 checks passed
@nono nono deleted the fix-couch-keepalive branch November 28, 2024 10:54
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