resolver: automatically reload /etc/resolv.conf

[vangyzen]

Summary

On each resolver query, use stat(2) to see if the modification time
of /etc/resolv.conf has changed. If so, reload the file and reinitialize
the resolver library. However, only call stat(2) if at least ten seconds
have passed since the last call to stat(2), since calling stat(2) on every
query could kill performance.

This new behavior is enabled by default. Add a "no-reload" option
to disable it.

Document this behavior and option in resolv.conf(5).

Test Plan

I manually tested this with a long-running interactive multi-threaded
resolver test client, while watching tcpdump and ktrace to verify that
stat(2) calls were rate-limited and that /etc/resolv.conf was reloaded.

[alfred]

10 seconds is a bit long, can it be 1 second?

[vangyzen]

I suppose 10 seconds is a bit long. I will try something much lower and re-run a benchmark to make sure the impact is not noticeable.

(I chose 10 seconds before adding the no-reload option, so it had to be appropriate for all cases.)

[vangyzen]

On second thought, I might change the option to "reload-delay:N" where N is the number of seconds and N=0 disables it entirely. This would make everyone happy.

[alfred]

In D3867#80115, @vangyzen wrote:

On second thought, I might change the option to "reload-delay:N" where N is the number of seconds and N=0 disables it entirely. This would make everyone happy.

Thank you for making it configurable.

1 second by default should be OK if I am understanding this patch.

[kp]

include/resolv.h

191 ↗

(On Diff #9314)

struct timespec<tab>conf_mtim. (Minor style remark)

lib/libc/resolv/res_state.c

69 ↗

(On Diff #9314)

It might be slightly better to rewrite this as

if ((statp->options & (RES_INIT|RES_NO_RELOAD)) != RES_INIT)
  return (res_state);

That reduces the indentation level a little, and it's (in my view at least) easier to follow what happens.
__res_state() does that as well.

share/man/man5/resolver.5

202 ↗

(On Diff #9314)

Was this a mistake? It's not mentioned in the commit message, and '(e.g., nameserver)' looks wrong to me.

[vangyzen]

resolver: automatically reload /etc/resolv.conf

On each resolver query, use stat(2) to see if the modification time
of /etc/resolv.conf has changed. If so, reload the file and reinitialize
the resolver library. However, only call stat(2) if at least two seconds
have passed since the last call to stat(2), since calling stat(2) on every
query could kill performance.

This new behavior is enabled by default. Add a "reload-period" option
to disable it or change the period of the test.

Document this behavior and option in resolv.conf(5).

Add a comma after "e.g." to appease igor.

[vangyzen]

In D3867#80116, @alfred wrote:

1 second by default should be OK if I am understanding this patch.

I chose two seconds for the default, since I test with whole-second resolution.

[vangyzen]

lib/libc/resolv/res_state.c

67 ↗

(On Diff #9335)

Each style has its pros and cons. I changed to your suggested style because it's the most common in FreeBSD code, and in this file.

share/man/man5/resolver.5

206 ↗

(On Diff #9335)

I failed to mention that this appeases igor.

Most style guides say the comma is more common or more correct. I tend to agree. If one were to write out "for example", one would need a comma, due to the slight pause in speech. I agree that it looks a bit odd, probably because ".," is uncommon.

[kp]

share/man/man5/resolver.5

206 ↗

(On Diff #9335)

Ah, I did not know that, so I've learned something today.

[vangyzen]

share/man/man5/resolver.5

206 ↗

(On Diff #9335)

Then it was a good day. :)

[wblock]

share/man/man5/resolver.5

179 ↗

(On Diff #9335)

Suggestion to simplify and combine these sentences:

The resolver checks the modification time of
.Pa /etc/resolv.conf
every
.Ar n
seconds.  If
.Pa /etc/resolv.conf
has changed, it is automatically reloaded.
Setting
.Ar n
to zero disables the file check.

(And then remove everything up to .El)

206 ↗

(On Diff #9335)

It turns out that a lot of people use e.g. and i.e. wrong. "For example" and "that is" are less ambiguous. The igor style check (-y) suggests just using the English versions.

I think the \& is a remnant from an older man page tool. Really, this should be:

.Pq for example, Sy nameserver

[bjk]

[trivia about mdoc inline]

share/man/man5/resolver.5

206 ↗

(On Diff #9335)

The zero-width space \& was needed when the '.' was the last character on the line, since those are treated as "end-of-sentence" markers by many doc toolchains, but this is not the end of the sentence. If it had been followed by a comma, as is required by our style, there would be no need for the zero-width space.

[vangyzen]

share/man/man5/resolver.5

179 ↗

(On Diff #9335)

I was trying to communicate more detail about the implementation, since that could be very important in some environments. After a good night's sleep and my second cup of coffee, however, I think your version is a more appropriate level of detail for this context. I just need to mention the default value of
.Ar n .
(Wouldn't it be neat if Phab were to support mdoc markup in these comments?)

206 ↗

(On Diff #9335)

I was just making igor run cleanly, but okay, I'll bite. ;)

Thanks, both of you, for your input.

[vangyzen]

Incorporate man page suggestions from wblock.