Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(146)

Issue 67730046: code review 67730046: net/http: graceful stop for http.Server

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 1 month ago by rcrowley
Modified:
10 years ago
Visibility:
Public.

Description

net/http: graceful stop for http.Server This patch adds a Close method to http.Server which waits for all open connections to be closed. A chan struct{} indicates to goroutines that they should stop. Busy connections close themselves after responding to the client if the channel is closed. bufio.Reader.Peek is used to detect idle connections earlier than http.Server.ReadTimeout typically allows. Fixes issue 4674.

Patch Set 1 #

Patch Set 2 : diff -r 70499e5fbe5b https://code.google.com/p/go #

Patch Set 3 : diff -r 3cf533be5e36 https://code.google.com/p/go #

Patch Set 4 : diff -r 3cf533be5e36 https://code.google.com/p/go #

Patch Set 5 : diff -r 3cf533be5e36 https://code.google.com/p/go #

Patch Set 6 : diff -r 3cf533be5e36 https://code.google.com/p/go #

Patch Set 7 : diff -r 3cf533be5e36 https://code.google.com/p/go #

Total comments: 22

Patch Set 8 : diff -r 3cf533be5e36 https://code.google.com/p/go #

Total comments: 2

Patch Set 9 : diff -r 3cf533be5e36 https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+210 lines, -3 lines) Patch
M src/pkg/net/http/serve_test.go View 1 2 3 4 5 6 7 1 chunk +101 lines, -0 lines 0 comments Download
M src/pkg/net/http/server.go View 1 2 3 4 5 6 7 8 9 chunks +109 lines, -3 lines 0 comments Download

Messages

Total messages: 23
rcrowley
Hello golang-codereviews@googlegroups.com (cc: bradfitz@golang.org), I'd like you to review this change to https://code.google.com/p/go
10 years, 1 month ago (2014-02-24 09:39:46 UTC) #1
dvyukov
What is the effect on performance? In particular I am interested in: $ go get ...
10 years, 1 month ago (2014-02-24 09:55:44 UTC) #2
rcrowley
On 2014/02/24 09:55:44, dvyukov wrote: > What is the effect on performance? > > In ...
10 years, 1 month ago (2014-02-24 18:20:01 UTC) #3
rcrowley
Wade Simmons pointed out that this patch will only Close the last net.Listener passed to ...
10 years, 1 month ago (2014-02-24 18:44:33 UTC) #4
rcrowley
On 2014/02/24 18:44:33, rcrowley wrote: > Wade Simmons pointed out that this patch will only ...
10 years, 1 month ago (2014-02-25 05:13:10 UTC) #5
dvyukov
On Mon, Feb 24, 2014 at 10:20 PM, <r@rcrowley.org> wrote: > On 2014/02/24 09:55:44, dvyukov ...
10 years, 1 month ago (2014-02-25 06:06:35 UTC) #6
rcrowley
> 5.62% slower with GOMAXPROCS=2 > > when/if we decide to submit it, let me ...
10 years, 1 month ago (2014-02-25 18:22:33 UTC) #7
bradfitz
https://codereview.appspot.com/67730046/diff/120001/src/pkg/net/http/server.go File src/pkg/net/http/server.go (right): https://codereview.appspot.com/67730046/diff/120001/src/pkg/net/http/server.go#newcode119 src/pkg/net/http/server.go:119: remove this blank line. the following is also guarded ...
10 years, 1 month ago (2014-02-25 19:15:10 UTC) #8
bradfitz
What if we make this opt-in and add a new field to Server and require ...
10 years, 1 month ago (2014-02-25 19:16:07 UTC) #9
rcrowley
This refactor addresses all of Brad's comments but not the proposal to make graceful stop ...
10 years, 1 month ago (2014-02-26 19:46:46 UTC) #10
rcrowley
On 2014/02/25 19:16:07, bradfitz wrote: > What if we make this opt-in and add a ...
10 years, 1 month ago (2014-02-26 19:52:11 UTC) #11
ioe
On Wednesday, February 26, 2014 8:52:11 PM UTC+1, r...@rcrowley.org wrote: > And while we're debating ...
10 years, 1 month ago (2014-02-26 20:25:30 UTC) #12
kisielk
On Wednesday, February 26, 2014 12:25:29 PM UTC-8, night...@googlemail.com wrote: > On Wednesday, February 26, ...
10 years, 1 month ago (2014-02-26 21:15:19 UTC) #13
rcrowley
On 2014/02/26 20:25:30, ioe wrote: > On Wednesday, February 26, 2014 8:52:11 PM UTC+1, mailto:r...@rcrowley.org ...
10 years, 1 month ago (2014-02-26 21:24:39 UTC) #14
rcrowley
> net.Listener uses Close() and so does httptest.Server I'm reading this as an argument in ...
10 years, 1 month ago (2014-02-26 21:28:28 UTC) #15
kisielk
On Wed, Feb 26, 2014 at 1:28 PM, <r@rcrowley.org> wrote: > net.Listener uses Close() and ...
10 years, 1 month ago (2014-02-26 21:34:52 UTC) #16
rcrowley
On 2014/02/26 21:34:52, kisielk wrote: > On Wed, Feb 26, 2014 at 1:28 PM, <mailto:r@rcrowley.org> ...
10 years, 1 month ago (2014-02-26 21:45:23 UTC) #17
bradfitz
Richard, I've sent out https://codereview.appspot.com/69260044 --- would that allow you do what you need without ...
10 years, 1 month ago (2014-02-26 22:27:18 UTC) #18
bradfitz
https://codereview.appspot.com/67730046/diff/130001/src/pkg/net/http/server.go File src/pkg/net/http/server.go (right): https://codereview.appspot.com/67730046/diff/130001/src/pkg/net/http/server.go#newcode913 src/pkg/net/http/server.go:913: if w.conn.server.stopping() { this accessing stoppignc without a lock, ...
10 years, 1 month ago (2014-02-26 22:49:18 UTC) #19
rcrowley
https://codereview.appspot.com/67730046/diff/130001/src/pkg/net/http/server.go File src/pkg/net/http/server.go (right): https://codereview.appspot.com/67730046/diff/130001/src/pkg/net/http/server.go#newcode913 src/pkg/net/http/server.go:913: if w.conn.server.stopping() { On 2014/02/26 22:49:19, bradfitz wrote: > ...
10 years, 1 month ago (2014-02-27 07:14:29 UTC) #20
dvyukov
On Thu, Feb 27, 2014 at 11:14 AM, <r@rcrowley.org> wrote: > > https://codereview.appspot.com/67730046/diff/130001/src/pkg/net/http/server.go > File ...
10 years, 1 month ago (2014-02-27 07:18:37 UTC) #21
rcrowley
On 2014/02/26 22:27:18, bradfitz wrote: > Richard, > > I've sent out https://codereview.appspot.com/69260044 --- would ...
10 years, 1 month ago (2014-02-27 07:43:31 UTC) #22
bradfitz
10 years ago (2014-03-11 17:54:41 UTC) #23
R=close

Not for Go 1.3, per comments on Issue 4674
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b