net: lookup: avoid creating a new goroutine if context is canceled and lookupIPAddr isn't actually resolving

[KeiichiHirobe]

Hello,

I've read

go/src/net/lookup.go

Lines 333 to 347 in 0aa14fc

	case <-ctx.Done():
	// Our context was canceled. If we are the only
	// goroutine looking up this key, then drop the key
	// from the lookupGroup and cancel the lookup.
	// If there are other goroutines looking up this key,
	// let the lookup continue uncanceled, and let later
	// lookups with the same key share the result.
	// See issues 8602, 20703, 22724.
	if r.getLookupGroup().ForgetUnshared(lookupKey) {
	lookupGroupCancel()
	go dnsWaitGroupDone(ch, func() {})
	} else {
	go dnsWaitGroupDone(ch, lookupGroupCancel)
	}
	ctxErr := ctx.Err()

and #22724.

I think It's preferable to avoid creating a new goroutine for waiting on a result whenever possible. If the context is canceled and lookupIPAddr isn't actually resolving, there's no need to wait for the result using a new goroutine.

This is how we can achieve this:

1. Add method ForgetNonFirstChan to internal/singleflight.Group, which signature is

// ForgetNonFirstChan tells the singleflight to forget about a channel if it is not
// for the first invocation for a key.
// Returns whether the channel was forgotten or not.
func (g *Group) ForgetNonFirstChan(key string, ch chan Result) bool 

2. At lookupIPAddr, when context canceled, call ForgetNonFirstChan and if it returns true, we don't need to wait for the result, and call cancelFn immediately without making a new goroutine.

For example, suppose there is a situation where the first call of LookupIPAddr takes a long time, and then calls for name resolution for the same hostname are made n times with a timeout of 5 seconds. Previously, even after 5 seconds had passed, n+1 goroutines were required, but after my modification, only one goroutine is required.

I wrote a draft implementation on Gerrit

FYI:
I changed the data structure that holds the channels from a slice to a map. This was done to improve the search(ForgetNonFirstChan needs searching) complexity from O(N) to O(1).

[gopherbot]

Change https://go.dev/cl/478895 mentions this issue: net: avoid creating a new goroutine if context is canceled and lookupIPAddr isn't actually resolving

[ianlancetaylor]

I can't see any reason for this to be a proposal, so taking it out of the proposal process.