TrueCloudLab/coredns
16
0
Fork 2
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions

middleware/proxy: remove singleinflight from dns (#717)

Browse source 
Singleinflight interferes with the health checking of upstream. If an
upstream would fail, singleinflight would mirror that error to to other
proxy *iff* multple identical queries would be inflight. This would lead
to marking *all* upstreams as bad, essentially collapsing multiple
upstreams into a SPOF. Clearly not what we want.

Singleinflight does have some nice properties, but I've opted to rip it
out entirely. Caching should almost (but not quite) as good.

Added a test case in test that uses 3 CoreDNS instances to reflect the
setup from #715.

Found another bug as well, where (when the policy would be nil), we
would always Spray even though we've found a healthy host.
This commit is contained in:
Miek Gieben 2017-06-08 16:06:14 +01:00 • committed by GitHub
parent 36396e94ab
commit 1c45e262f5
3 changed files with 83 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

32
middleware/proxy/dns.go
View file

@ -5,7 +5,6 @@ import (
"net"
"time"
"github.com/coredns/coredns/middleware/pkg/singleflight"
"github.com/coredns/coredns/request"
"github.com/miekg/dns"
@ -13,7 +12,6 @@ import (
type dnsEx struct {
Timeout time.Duration
group *singleflight.Group
Options
}
@ -26,7 +24,7 @@ func newDNSEx() *dnsEx {
}
func newDNSExWithOption(opt Options) *dnsEx {
return &dnsEx{group: new(singleflight.Group), Timeout: defaultTimeout * time.Second, Options: opt}
return &dnsEx{Timeout: defaultTimeout * time.Second, Options: opt}
}
func (d *dnsEx) Protocol() string { return "dns" }
@ -65,30 +63,14 @@ func (d *dnsEx) Exchange(ctx context.Context, addr string, state request.Request
}
func (d *dnsEx) ExchangeConn(m *dns.Msg, co net.Conn) (*dns.Msg, time.Duration, error) {
t := "nop"
if t1, ok := dns.TypeToString[m.Question[0].Qtype]; ok {
t = t1
}
cl := "nop"
if cl1, ok := dns.ClassToString[m.Question[0].Qclass]; ok {
cl = cl1
}
start := time.Now()
// Name needs to be normalized! Bug in go dns.
r, err := d.group.Do(m.Question[0].Name+t+cl, func() (interface{}, error) {
return exchange(m, co)
})
r1 := r.(dns.Msg)
r, err := exchange(m, co)
rtt := time.Since(start)
return &r1, rtt, err
return r, rtt, err
}
// exchange does *not* return a pointer to dns.Msg because that leads to buffer reuse when
// group.Do is used in Exchange.
func exchange(m *dns.Msg, co net.Conn) (dns.Msg, error) {
func exchange(m *dns.Msg, co net.Conn) (*dns.Msg, error) {
opt := m.IsEdns0()
udpsize := uint16(dns.MinMsgSize)
@ -109,7 +91,7 @@ func exchange(m *dns.Msg, co net.Conn) (dns.Msg, error) {
dnsco.Close()
if r == nil {
return dns.Msg{}, err
return nil, err
}
return *r, err
return r, err
}