Skip to content

[tailscale] net/http/internal/http2: let request-write goroutines exit early - #173

Open
bradfitz wants to merge 1 commit into
tailscale.go1.27from
bradfitz/h2_writereq_goroutine
Open

[tailscale] net/http/internal/http2: let request-write goroutines exit early#173
bradfitz wants to merge 1 commit into
tailscale.go1.27from
bradfitz/h2_writereq_goroutine

Conversation

@bradfitz

Copy link
Copy Markdown
Member

The goroutine spawned per RoundTrip to write the request previously
parked until the stream ended, even after the request was fully sent,
just to wait for the stream-end events and run cleanupWriteRequest.
For clients with many concurrent long-lived response streams (long
polls, event streams), that's a parked goroutine and its stack per
stream doing nothing, which adds up to a large fraction of such a
client's memory use.

Once the request is fully sent, detach: the goroutine exits, and
cleanupWriteRequest instead runs (on a short-lived goroutine) from
whichever stream-end event fires first. Both stream-end events (peer
half-close and abort) are raised under cc.mu, so a mutex-guarded flag
gives exactly-once cleanup dispatch. Request context cancellation is
watched via context.AfterFunc, and ResponseHeaderTimeout is enforced
by a time.AfterFunc timer armed at detach and disarmed when response
headers arrive. Only the deprecated Request.Cancel channel still
requires a parked goroutine to watch it, so requests using it keep
the previous behavior.

Intended for upstreaming; carried in the Tailscale fork until then.

Updates tailscale/corp#29053

…t early

The goroutine spawned per RoundTrip to write the request previously
parked until the stream ended, even after the request was fully sent,
just to wait for the stream-end events and run cleanupWriteRequest.
For clients with many concurrent long-lived response streams (long
polls, event streams), that's a parked goroutine and its stack per
stream doing nothing, which adds up to a large fraction of such a
client's memory use.

Once the request is fully sent, detach: the goroutine exits, and
cleanupWriteRequest instead runs (on a short-lived goroutine) from
whichever stream-end event fires first. Both stream-end events (peer
half-close and abort) are raised under cc.mu, so a mutex-guarded flag
gives exactly-once cleanup dispatch. Request context cancellation is
watched via context.AfterFunc, and ResponseHeaderTimeout is enforced
by a time.AfterFunc timer armed at detach and disarmed when response
headers arrive. Only the deprecated Request.Cancel channel still
requires a parked goroutine to watch it, so requests using it keep
the previous behavior.

Intended for upstreaming; carried in the Tailscale fork until then.

Updates tailscale/corp#29053

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I5500c691194457195b0869c1612bf93718ca96c1
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.

1 participant