[squid-dev] [PATCH] Happy Eyeballs: Use each fully resolved destination ASAP

[Amos Jeffries]

Okay.  +1 on the code now


On 25/05/17 08:42, Alex Rousskov wrote:
> On 05/24/2017 09:04 AM, Amos Jeffries wrote:
>> For the 1st destination ps_state::handlePath() calls noteDestination()
>> which results in the entire TunnelState::startConnecting() /
>> FwdState::startConnectionOrFail() sequence happening synchronously
>> before returning to ps_state::handlePaths() and peerSelectDnsResults()
>> for the 2nd path.
> That is correct, although the phrasing "ENTIRE sequence" implies some
> significant delays that are usually not there. The "sequence" usually
> stops "soon" because it starts an async job (e.g., a PeerConnector or a
> Client job).
>
>
>> There do not seem to be any performance gains from this redesign for
>> transactions where there are less than 3 upstream servers.
> This is both incorrect and largely irrelevant:
>
> * Incorrect: The new code makes forwarding faster in cases where there
> are two (or more) destinations and the second destination requires a DNS
> lookup -- the new code will start using the first destination sooner
> and, in typical cases, will finish forwarding sooner. See my earlier
> specific example for an illustration.
>
> * Irrelevant: This performance gain is just a nice side effect. The
> primary goal is to facilitate Happy Eyeballs (which requires this
> parallelization and more).

Are you aware of the socket expense issue inherent in Happy Eyeballs? 
Each hop along the path generating M(2+2^N) outbound sockets where M is 
the number of HTTP carrying protocols and N the number of hops between 
it and the end-client - the protocol mix has gone from (M=2,N=4) when it 
was first identified to more like (M=6,N=6) these days.

I really do not think that is something to be aspiring towards creating. 
Parallelism and similar things within the internal processing yes, the 
"Happy Eyeballs" algorithm specifically no.


>
>> The synchronous processing may make the one transaction
>> reach connection faster, but that is just taking CPU cycles from /
>> slowing other transactions.
> The patch parallelizes second DNS lookup and first HTTP transaction
> execution. That parallelization is not about CPU cycles but about the
> time the first HTTP transaction spends waiting for (blocked on) that
> second (and usually unused) DNS lookup in the old code. The patched code
> does not wait/block. Again, the gains from this optimization are not the
> goal, but some environments will see them.
>
>
>> It might be worth initiating that noteDestination() sub-path as async
>> step to avoid the remaining delay.
> Yes, and my second thread email stated that much. However, the reason
> for making that call async is not performance optimization (async calls
> make performance worse, not better!) or delays. The reason is to shorten
> the call stack, reducing the number of balls in the air and localizing bugs.
>

I would be interested if there are any measurements on the above two 
points but its not worth blocking the patch on (sorry, bad pun intended 
;-P ).

Amos