Re: [PATCH] schedule connect timeouts via eventAdd

Alex Rousskov <rousskov_at_measurement-factory.com> writes:
> On 01/24/2013 05:57 AM, Rainer Weikusat wrote:
>> Amos Jeffries <squid3_at_treenet.co.nz> writes:
>>> On 24/01/2013 1:50 p.m., Alex Rousskov wrote:
>>>> Is storing 60'000 events instead of 100 acceptable? I kind of doubt it
>>>> is... :-(
>
>>> I agree.
>
>> As a somewhat more general remark to that: There are two obvious
>> alternative implementations, namely
>>
>> - use a timing wheel, ie, a 'circular' hash table where new events are
>> linked onto the list at (head + interval) % wheel_size. That would
>> be the traditional kernel implementation of this facility (That's
>> from what I remember from reading about that. I've never used it
>> myself)
>>
>> - use a differently implemented priority queue. The usual 'obvious
>> choice' would be a binary heap stored in an array
>
> Since
>
> * there are well-justified concerns about RAM or performance overhead of
> event-based timeouts with the current event queue,

The concern I originally wrote about was whether it would be better to
let the event-based timeout expire or to cancel it instead. The 'RAM
overhead' of using the event queue would be the memory needed for an
ev_entry object. That's 48 bytes, compared to the size of the
ConnOpener object figuring at 112 bytes. Consequently, this would need
about 1.43 times the amount of memory --- IMHO, hardly a
'catastrophic' increasement. And I wouldn't worry very much about the
'absolute' performance cost caused by the linear searches, either, at
least not until there is more data than 'educated guesses' available.

Eg, the 'standard' racoon 'schedular' (timer facility) uses a linked
lists (actually, racoon uses linked lists for just about everything)
and a lot of 'serious' commercial products, like so-called 'high-end
firewall appliances' use that for 'VPN support'.

> * replacing legacy event queue with a better one is a serious
> stand-alone project, with no "obviously best" replacement available in
> STL, and

That's easily doable in an afternoon and completely straight-forward
and self-contained (provided that the queue is only manipulated via
the public interface). An 'example' C implementation I happen to have
lying around here (used in the racoon variant used by my employer,
handling installations with up to 3500 users) figures at 95
LOC. An STL implementation is also available but given the miniscule
size of this, IMO hardly worth using.

I'd volunteer to do that since I've implemented numerous of these
beasts over the course of the last odd eight years or so, however, I'm
convinced that the present implementation will be good enough and
fixing the ConnOpener code seems more important to me.

> * we need to fix ConnOpener ASAP,
>
> I suggest that we reverse the course and focus on fd-based
> implementation of timeouts instead. It will be efficient, and we can
> make it correct.

Technically, yes, but this would then require keeping the temporary Fd
open during a scheduled 'delayed connect retry' and move the adjusted
timeout to the new temporay Fd once the connection for the next
connect retry has been opened[*]. This will also require distributing
the timeout handling code throughout a sizable part of the file which
is somewhat error-prone, as evidenced by the shortcomings of the
present code. And the fd-based timeout code does linear searches of
the FD table in order to determine when a timeout occurred which is
also not exactly a 'scalable' approach.

[*] This is not quite true: It would also be possible to use dup2 to
associated the new 'socket object' with the old file descriptor after
the open.
Received on Thu Jan 24 2013 - 18:16:48 MST