Skip to content

/go

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

net/http: add Transport.MaxIdleConns limit #15461

Closed
bradfitz opened this issue Apr 27, 2016 · 3 comments

Comments

Assignees

@bradfitz bradfitz

Labels
FrozenDueToAge
Projects
None yet
Milestone
  Go1.7
3 participants
@bradfitz @adg @gopherbot
@bradfitz
Copy link
Member

commented Apr 27, 2016

There's been a long-standing TODO in net/http.Transport that says:

type Transport struct {
....
    // TODO: tunable on global max cached connections
....

This bug is about that.

We already have Transport.MaxIdleConnsPerHost, but it's been requested by many (and again recently) that we have a Transport.MaxIdleConns for any host.

In the process, I also noticed that we're not aggressively removing cached connections when the peer server disconnects. We only lazily clean them up from the idle structures, which means we can hold onto memory for the bufio.Readers/Writers for a long time, wasting memory.

/cc @adg

@bradfitz bradfitz self-assigned this Apr 27, 2016

@bradfitz bradfitz added this to the Go1.7 milestone Apr 27, 2016

@adg

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Apr 27, 2016

So MaxIdleConns would limit the total number of idle connections across all hosts? SGTM.
@gopherbot

This comment has been minimized.

Sign in to view
Copy link

commented Apr 27, 2016

CL https://golang.org/cl/22492 mentions this issue.

gopherbot pushed a commit that referenced this issue Apr 27, 2016

@bradfitz
net/http: remove idle transport connections from Transport when serve… 
…r closes

Previously the Transport would cache idle connections from the
Transport for later reuse, but if a peer server disconnected
(e.g. idle timeout), we would not proactively remove the *persistConn
from the Transport's idle list, leading to a waste of memory
(potentially forever).

Instead, when the persistConn's readLoop terminates, remote it from
the idle list, if present.

This also adds the beginning of accounting for the total number of
idle connections, which will be needed for Transport.MaxIdleConns
later.

Updates #15461

Change-Id: Iab091f180f8dd1ee0d78f34b9705d68743b5557b
Reviewed-on: https://go-review.googlesource.com/22492
Reviewed-by: Andrew Gerrand <adg@golang.org>
2e30218
@gopherbot

This comment has been minimized.

Sign in to view
Copy link

commented Apr 30, 2016

CL https://golang.org/cl/22655 mentions this issue.

@gopherbot gopherbot closed this in 81b2ea4 May 1, 2016

@golang golang locked and limited conversation to collaborators May 1, 2017

@gopherbot gopherbot added the FrozenDueToAge label May 1, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.