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 connection reuse in splithttp HTTP/1.1 #3485

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

mmmray
Copy link
Collaborator

@mmmray mmmray commented Jun 30, 2024

Fix two bugs:

  • When the connection pool contains more than 5 dead connections, the write fails after 5 tries and is aborted. Instead, pull infinite amount of connections from the pool and when the pool is empty, fail the write on the first fresh connection.

  • When the write is retried, the request body is already drained, and the upload request will contain no payload at all. Fix this by stringifying the request eagerly.

Also explicitly set content-length so that transfer-encoding: chunked is avoided. This really doesn't seem to matter to any CDN at all though.

In a future version, there should be an explicit mechanism to read responses asynchronously and retry. This will certainly increase reliability further. However, if you actually care about reliability you should use h2

Fix two bugs:

* When the connection pool contains more than 5 dead connections, the
  write fails after 5 tries and is aborted. Instead, pull infinite
  amount of connections from the pool and when the pool is empty, fail
  the write on the first fresh connection.

* When the write is retried, the request body is already drained, and
  the upload request will contain no payload at all. Fix this by
  stringifying the request eagerly.

Also explicitly set content-length so that transfer-encoding: chunked is
avoided. This really doesn't seem to matter to any CDN at all though.
@RPRX
Copy link
Member

RPRX commented Jul 1, 2024

这样的 bug fix 准备好后,你可以直接 Squash and merge

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