cancelling requests due to max_request_time

[Frank Cusack]

On January 16, 2007 1:29:31 PM +0100 Alan DeKok <aland at deployingradius.com> 
wrote:
> Frank Cusack wrote:
>
>>> Is that right?  There are bugs there (besides pthread_cancel())?
>
>   Perhaps.
>
>>>  I see
>>> that if delete_block_requests is no, pthread_cancel() doesn't get
>>> called and instead STOP_NOW is set, so that looks good since usu. a
>>> request would be blocked on I/O, not compute, so waiting for a module
>>> to complete some I/O shouldn't be an issue.
>
>   Except for issues like DNS, or SQL servers being down, or LDAP being
> non-responsive for minutes.  But yes, it will mostly work.

Those are the exact I/O issues that would typically be the problem.
By "not an issue" I mean not an issue for FR to just wait for the
module to timeout its request, return, and then the STOP_NOW flag
to take effect, as opposed to cancelling the operation now; because
there's not going to be load on FR while it's just blocked on I/O.
ie, NOT having delete_blocked_requests shouldn't cause a cpu load
problem for FR.

>>> I would suggest simply removing the delete_blocked_requests option
>>> (always use "no").  The STOP_NOW flag looks good enuf.
>
>   OK.
>
>> Thought more about this.  Maybe a global cancel handler which calls
>> into a new .cancel method of each module.  If the module doesn't have
>> this function it won't be cancelable.  Hey I like that a lot.
>
>   That's easy enough to do.  But if a module is cancellable, odds are it
> won't be blocking on anything...

If a module is cancellable, it has a cancel handler that would close
all open fd's and release mutexes.  That's the point of the handler,
to cleanly release those resources so it can be canceled.

>> Just need to ensure delete_blocked_requests isn't actually buggy.
>
>   Now that I think about it, the request has to be deleted from the
> "live" list, and placed onto a "dead" list.  That's so its memory isn't
> leaked.
>
>   I spent some time trying to come up with a good & easy solution, but
> it never went anywhere.  The "packet list" stuff in CVS head is a start,
> but not completely fleshed out.

Might even be able to do a lock-free implementation.

-frank