client: allow re-using connections in more cases
I am seeing a bug where connections with a Streamable HTTP client are not reused at all. This makes performance terrible, since each request establishes a new TCP connection. I was able to root cause this down to behavior of the `processStream` function. This sets up a bufio.NewScanner, and waits to get data. Once we get a message, we abort and *close the connection*. At this point, there are two paths: * The bufio reader read the *full* response body to EOF, and then we closed. This case is fine; we can re-use the idle connection. * The bufio reader *did not* real the full response body. It may have read the entire actual data that *would be sent*, but since its streaming happened to not yet be delivered. In this case, we call `body.Close()` before an EOF, which causes the HTTP client to not attempt to re-use the connection. In my case, the server I am using reliable sends the response over 2 TCP packets (one with the event, and the other with the closure of the `chunked` encoding (`0`)). This causes the body to never be fully read. This PR fixes that case by unconditionally reading the full body before we close the connection. With this change, I am able to reliably re-use connections for future requests.
This commit is contained in:
parent
6848e36ef0
commit
b012d0824e
1 changed files with 5 additions and 1 deletions
|
|
@ -1835,7 +1835,11 @@ func (c *streamableClientConn) checkResponse(requestSummary string, resp *http.R
|
|||
// indicating if the connection was closed by the client. If resp is nil, it
|
||||
// returns "", false.
|
||||
func (c *streamableClientConn) processStream(ctx context.Context, requestSummary string, resp *http.Response, forCall *jsonrpc.Request) (lastEventID string, reconnectDelay time.Duration, clientClosed bool) {
|
||||
defer resp.Body.Close()
|
||||
defer func() {
|
||||
// Drain any remaining unprocessed body. This allows the connection to be re-used after closing.
|
||||
io.Copy(io.Discard, resp.Body)
|
||||
resp.Body.Close()
|
||||
}()
|
||||
for evt, err := range scanEvents(resp.Body) {
|
||||
if err != nil {
|
||||
if ctx.Err() != nil {
|
||||
|
|
|
|||
Loading…
Reference in a new issue