Add support for DNS config hot-reload #4875
Conversation
ShimmerGlass
force-pushed
the
dns-config-reload
branch
from
47c8c20
to
9b34006
Compare
There was a problem hiding this comment.
Looks great, being able to hot reload DNS config is a great feature when performing changes on a Consul agent while not degrading qOS on a DNS resolver, especially when it is a key in the infrastructure
Definitely deserves a look to increase operations in a real life cluster!
|
@pearkes @mkeeler do you think it could be integrated? The change is big in number of lines, but not that complex and would allow people using Consul DNS to limit the times agent do restart to take into account DNS parameters (such as cache settings) It could be a huge win in terms of operability (since DNS is a very critical piece of infrastructure and limiting its downtime is really critical) |
ShimmerGlass
force-pushed
the
dns-config-reload
branch
from
9b34006
to
75a6dc5
Compare
ShimmerGlass
force-pushed
the
dns-config-reload
branch
from
75a6dc5
to
0664529
Compare
ShimmerGlass
force-pushed
the
dns-config-reload
branch
from
0664529
to
780dc30
Compare
|
@Aestek Can you rebase this? it seems it still has conflicts... |
ShimmerGlass
force-pushed
the
dns-config-reload
branch
from
780dc30
to
46d7174
Compare
The DNS config parameters `recursors` and `dns_config.*` are now hot reloaded on SIGHUP or `consul reload` and do not need an agent restart to be modified. Config is stored in an atomic.Value and loaded at the beginning of each request. Reloading only affects requests that start _after_ the reload. Ongoing requests are not affected. To match the current behavior the recursor handler is loaded and unloaded as needed on config reload.
ShimmerGlass
force-pushed
the
dns-config-reload
branch
from
46d7174
to
eac71a4
Compare
There was a problem hiding this comment.
This looks good to me. Everything I would have thought of looks to have already been handled.
One question I have is whether or not you are already running with this and if so have there been a noticeable performance degradation with the change? Doing an atomic Load once per request should be very minimal especially when stores are not happening often. Just curious if you have checked it out under load.
|
@Aestek Any feedback on the question from @mkeeler about any testing you might have done for this change? |
|
@mkeeler @pearkes woops sorry didnt see the comment. |
|
@Aestek thanks for the update. |
There was a problem hiding this comment.
I think we actually missed adding this to https://www.consul.io/docs/agent/options.html#reloadable-configuration as well, we should do that.
This PR adds support for hot-reloading
recursorsanddns_config.*settings on SIGHUP orconsul reload.DNS settings currently need an agent restart to be modified. This makes changing the configuration of consul agents queried by our DNS servers for
.consuldomains hard because they handle a lot of requests and cannot be easily restarted. The TTL and SOA settings are especially important due to their impact on performance.This change stores the configuration in an
atomic.Valueand loads it at the beginning of each request. Reloading only affects requests that start after the reload. Ongoing requests are not affected. To match the current behavior the recursor handler is loaded and unloaded as needed on config change.