Re: dns_timeout and dns_retransmit_interval in ms

 On Tue, 15 Mar 2011 14:14:02 +0200, Tsantilas Christos wrote:
> So, is it OK to use the 30 seconds as the default dns_timeout value?
> If not objection I will commit this patch later today ....
>
>
> On 03/14/2011 06:35 PM, Alex Rousskov wrote:
>
>>> +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));
>
> OK fixed.
>
>>
>> We can also use T_SECOND_STR and T_MILLISECOND_STR in the above.
> I did not use them because of the missing "s" at the end of these
> strings ("second" not "seconds" etc....)
>
>>
>>> - 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
>
> done
>
>>
>> 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.
>
> done
>
> On 03/15/2011 01:25 AM, Amos Jeffries wrote:
>>
>> Yes. We MUST even. People use the cachemgr output these dump
>> routines
>> flow into as copy-n-paste sources for other squid.conf and
>> tutorials.
>>
>>>
>>> if (var % 1000)
>>> storeAppendPrintf(entry, "%s %d milliseconds\n", name, (int) var);
>>> else
>>> storeAppendPrintf(entry, "%s %d seconds\n", name, (int)
>>> (var/1000));
>>
>> Please also avoid the 32-bit cast by using %"PRIu64" for all
>> unsigned
>> 64-bit display.
>
> done
>
>>
>> Also, conversion to the largest whole time period (second, minute,
>> hour,
>> day etc as supported by the parser) would be good for user education
>> through the display defaults.
>
> I did not change this one. I do not think it is something important
> because it is unusual to have big time periods, in the cases where
> the
> user should use milliseconds...

 Fine. Just an idea.

 Amos
Received on Tue Mar 15 2011 - 21:19:32 MDT