Re: [PATCH] Move timeout handling from ConnOpener::connect to ConnOpener::timeout

On 25/01/2013 5:34 p.m., Alex Rousskov wrote:
> On 01/24/2013 06:37 PM, Amos Jeffries wrote:
>
>>>> /* When one connection closes, give accept() a chance, if need be */
>>>> Comm::AcceptLimiter::Instance().kick();
>> This one might kick us. (sorry bad pun).
> Heh! AFAICT, this kick() might deprive us from an FD slot later if we
> were using the last FD available (and now closing it because of the
> connect failures that we are hoping to overcome on a retry). I do not
> think this is a problem because ConnOpener will handle the corresponding
> conn_openex() "FD limit" error as any other. After all, ConnOpener may
> run out of FDs even if we did not kick any pending accepts.
>
> Did I miss any bad side effects of this kick()?

I was referring to the opposite direction - potential problems in the
case where a large group of FD would fail doing ConnOpener at roughly
the same time but not do the kick(). Leaving queued clients at more risk
of timing out on their accept()'s until something else triggered it.

>
>> IIRC it was just the isOpen() and F->closing() logics that were the
>> problem. If that has gone away like it seems to I have no issue with
>> using comm_close() other than a token hand waving in the direction of
>> performance.
> Sounds good. Since this is a rare failure case, I do not think we need
> to optimize it at the expense of code quality.
>
> If no other side effects are spotted, and I have the time, I will try
> comm_close(), which will remove one more instance of direct fd_table
> manipulation, and a nasty/complex one!

... followed up on the new thread where you posted the patch.

Amos
Received on Fri Jan 25 2013 - 04:51:15 MST