[squid-dev] [RFC] dns_wait_for_all

[Alex Rousskov]

On 09/20/2016 04:41 AM, Amos Jeffries wrote:
> On 16/09/2016 3:35 a.m., Alex Rousskov wrote:
>> On 09/15/2016 03:50 AM, Amos Jeffries wrote:
>>> The serverDestinations not changing (yet).

>> I am pretty sure we have to change that field to implement
>> dns_wait_for_all functionality correctly. For example:
>>
>> * we need to make its updates (from different jobs!) safe, with
>> easy-to-trace debugging.
>>
>> * we need to support an EOF-like semantics to signal whether more
>> destinations may still be provided.


> I think you are mistaking serverDestinations as being part of the DNS
> operations. It is not.

I am not thinking in those terms. serverDestinations today is populated
by peer selection code, based on DNS lookups (including DNS cache
lookups). Those DNS lookups create two problems that dns_wait_for_all is
solving (i.e., waiting for the second DNS query in each address
resolution transaction and waiting for all peer address resolution
transactions).


> serverDestinations is state in the FwdStateData 'Job', the IP addresses
> used by it have to go through the peer selection 'Job' processing and
> related access controls applied to whatever comes out of an ipcache
> lookup 'Job'.

Agreed. serverDestinations state is shared among peer selection and
FwdState code. Today, the sharing is sequential: After peer selection
fills serverDestinations, FwdState starts using serverDestinations. The
sequential nature of this approach creates problems. Dns_wait_for_all
solves some of them.


> DNS lookups are buried 3+ 'Job' layers away on the other side of
> ipcache. All they do is feed that cache with data.

Yes, there are many layers.

The DNS code itself does not quite feed the cache (it just sends an
answer to the requester which happens to be the cache), and the cache is
not quite a layer: A fresh DNS answer is given to both the cache and the
ipcache_nbgethostbyname() callback. There might be some unwelcomed
dependencies in the code that require that fresh answer to be cached,
but they should not exist and they are unimportant when we discuss high
level concepts.


> If you are intending to alter serveDestinations operations, then please
> dont call the option dns_* . It is a tcp_outgoing_* thing.

I do not recommend using the tcp_outgoing name prefix because the
directive has nothing to do with TCP (and everything to do with DNS).
However, I am not going to fight you on this -- we usually disagree on
how things should be named, and there usually are not enough formal
arguments that can be made to support a given name. If nobody speaks up,
you will be able to rename the directive however you want. I am
certainly not a big fan of dns_wait_for_all!

Needless to say, the directive name should reflect what the feature does
from Squid admin point of view (the code changes implementing the
feature are irrelevant when it comes to naming). The proposed feature
determines whether Squid waits for certain DNS lookups when forwarding
transactions to peers.


Cheers,

Alex.