Enable full dns compression #4131
Conversation
This reverts commit aeac058.
|
@mkeeler Hello, since you reviewed my previous DNS patch, you may have a look, this is a small change in Consul codebase itself, but I spent quite some time on patching tricky bugs in DNS library itself. Here are the result of this change: #4071 (comment) This change has strong impacts on products such as HaProxy performing DNS discovery with SRV records as it allows DNS to work on larger clusters (more than 50% of improvements) This patch was difficult to land since I had first to fix DNS library miekg/dns#668 but it will really improve the situation. |
There was a problem hiding this comment.
@pierresouchay The PR looks good. Just the one very minor addition if you could.
Also for the sake of documenting it, I also reviewed the changes in miekg/dns from v1.0.4 to v1.0.7 and they also look good including your updates to that library.
agent/dns.go
Outdated
| @@ -758,9 +755,9 @@ func (d *DNSServer) trimTCPResponse(req, resp *dns.Msg) (trimmed bool) { | |||
| // Beyond 2500 records, performance gets bad | |||
| // Limit the number of records at once, anyway, it won't fit in 64k | |||
| // For SRV Records, the max is around 500 records, for A, less than 2k | |||
| truncateAt := 2048 | |||
| truncateAt := 4096 | |||
There was a problem hiding this comment.
Could you update the comments here for why the truncation values of 4096/1024 were chosen.
There was a problem hiding this comment.
@mkeeler DONE
Will fix #4071
Since dns computation Len() has been fixed in 1.0.7 https://github.com/miekg/dns
We can now compute Len() on compressed messages safely, event when size of response overflows 14bits (aka 16k)
Thus, on large TCP responses, we will send around 64k of data for large clusters instead of far less.
It will allow several services relying on SRV records to use more backends on large clusters