Re: dns_timeout and dns_retransmit_interval in ms

On 03/12/2011 02:00 PM, Tsantilas Christos wrote:
> Convert dns_timeout and dns_retransmit_interval configuration options to
> use millisecond resolution.
>
> One second resolution is too coarse for small timeouts in
> delay-sensitive environments, especially when a retransmit, bypass, or
> another corrective action is available and is likely to produce a
> positive outcome. In DNS world specifically, most timeouts are measured
> in milliseconds.

> +static void
> +dump_time_msec(StoreEntry * entry, const char *name, uint64_t var)
> +{
> + storeAppendPrintf(entry, "%s %d seconds and %d milliseconds\n", name, (int) (var/1000), (int) (var % 1000));
> +}

I doubt we are consistent with this everywhere, but should we try to
dump options in a format compatible with squid.conf syntax? If yes, we
could do something like this (at least):

if (var % 1000)
    storeAppendPrintf(entry, "%s %d milliseconds\n", name, (int) var);
else
    storeAppendPrintf(entry, "%s %d seconds\n", name, (int) (var/1000));

We can also use T_SECOND_STR and T_MILLISECOND_STR in the above.

> - commSetTimeout(vc->fd, Config.Timeout.idns_query, NULL, NULL);
> + const int timeout = (Config.Timeout.idns_query % 1000 ?
> + Config.Timeout.idns_query + 1000 : Config.Timeout.idns_query) / 1000;
> +
> + commSetTimeout(vc->fd, timeout, NULL, NULL);

Perhaps add a source comment explaining that while we are forced to
convert to seconds here, the exact timeout will be checked in
idnsCheckQueue()?
// Comm needs seconds but idnsCheckQueue() will check the exact timeout

Please consider typedef-ing uint64_t as time_msec and using time_msec
type as needed. This will help with identifying time-related values in
the future.

None of the above polishing touches require resubmission of the patch.

I did not find any other problems with the latest version of the patch.
I am not excited about having seconds and milliseconds co-existing as
easily-confused integer types, but I can leave with that, at least until
there is more code using milliseconds and converting them, eventually,
into timeval anyway.

+0

Thank you,

Alex.
Received on Mon Mar 14 2011 - 16:35:37 MDT