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

Issue 154610044: code review 154610044: net: if a DNS lookup times out, forget that it is in flight (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 5 months ago by iant
Modified:
9 years, 5 months ago
Reviewers:
bradfitz
CC:
bradfitz, mikio, golang-codereviews, r, rsc
Visibility:
Public.

Description

net: if a DNS lookup times out, forget that it is in flight Before this CL, if the system resolver does a very slow DNS lookup for a particular host, all subsequent requests for that host will hang waiting for that lookup to complete. That is more or less expected when Dial is called with no deadline. When Dial has a deadline, though, we can accumulate a large number of goroutines waiting for that slow DNS lookup. Try to avoid this problem by restarting the DNS lookup when it is redone after a deadline is passed. This CL also avoids creating an extra goroutine purely to handle the deadline. No test because we would have to simulate a slow DNS lookup followed by a fast DNS lookup. Fixes issue 8602.

Patch Set 1 #

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

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

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

Total comments: 1

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

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -28 lines) Patch
M src/net/lookup.go View 1 2 3 4 2 chunks +28 lines, -23 lines 0 comments Download
M src/net/singleflight.go View 1 2 3 chunks +61 lines, -5 lines 0 comments Download

Messages

Total messages: 8
iant
Hello bradfitz (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
9 years, 5 months ago (2014-10-22 16:07:53 UTC) #1
iant
I'm OK with postponing this to 1.5 if that seems safer.
9 years, 5 months ago (2014-10-22 16:08:23 UTC) #2
mikio
> I'm OK with postponing this to 1.5 if that seems safer. i'm fine either ...
9 years, 5 months ago (2014-10-24 08:08:15 UTC) #3
bradfitz
I was already uncomfortable mirroring singleflight (which exists elsewhere) into the net package without it ...
9 years, 5 months ago (2014-10-24 16:21:12 UTC) #4
iant
On 2014/10/24 08:08:15, mikio wrote: > > > This CL also avoids creating an extra ...
9 years, 5 months ago (2014-10-24 19:00:53 UTC) #5
bradfitz
LGTM But I'll let you+others decide on Go 1.4 vs Go 1.5.
9 years, 5 months ago (2014-10-24 20:23:56 UTC) #6
iant
I'm going to go ahead and submit. The code still looks OK to me, and ...
9 years, 5 months ago (2014-10-27 15:46:03 UTC) #7
iant
9 years, 5 months ago (2014-10-27 15:46:21 UTC) #8
*** Submitted as https://code.google.com/p/go/source/detail?r=3d68989bd1ea ***

net: if a DNS lookup times out, forget that it is in flight

Before this CL, if the system resolver does a very slow DNS
lookup for a particular host, all subsequent requests for that
host will hang waiting for that lookup to complete.  That is
more or less expected when Dial is called with no deadline.
When Dial has a deadline, though, we can accumulate a large
number of goroutines waiting for that slow DNS lookup.  Try to
avoid this problem by restarting the DNS lookup when it is
redone after a deadline is passed.

This CL also avoids creating an extra goroutine purely to
handle the deadline.

No test because we would have to simulate a slow DNS lookup
followed by a fast DNS lookup.

Fixes issue 8602.

LGTM=bradfitz
R=bradfitz, mikioh.mikioh
CC=golang-codereviews, r, rsc
https://codereview.appspot.com/154610044
Sign in to reply to this message.

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