On Mon, Jan 24, 2022 at 10:29 AM Ruediger Pluem <rpl...@apache.org> wrote:
>
> On 1/24/22 12:43 AM, Graham Leggett wrote:
> > On 22 Jan 2022, at 15:01, Yann Ylavic <ylavic....@gmail.com> wrote:
> >
> >>> @@ -268,12 +268,14 @@ struct timeout_queue {
> >>> /*
> >>> * Several timeout queues that use different timeouts, so that we always
> >>> can
> >>> * simply append to the end.
> >>> + * read_line_q uses vhost's TimeOut FIXME - we can use a short
> >>> timeout here
> >>
> >> I think we need a better interaction with mod_reqtimeout if we add
> >> client side read timeouts in the MPM.
> >> For instance we document handshake=5 in mod_reqtimeout and it does not
> >> fit the fixed s->timeout here (handshake= is disabled by default for
> >> compatibility because it was added late(ly) in 2.4, but if it's
> >> configured we should honor it in the MPM too).
> >>
> >> Not sure how to do that but the single timeout per timeout_queue model
> >> is probably not the right one here.
> >> Maybe we could queue a timer event based on the actual connection's
> >> socket timeout?
> >
> > The present approach is to extend the existing model of queues in the MPM
> > from two queues to three, I don’t want to make too many changes to the
> > existing structure of the event MPM at this stage if I can get away with it.
> >
> > In theory, there should be an AcceptTimeout directive alongside Timeout and
> > KeepaliveTimeout that mod_reqtimeout should defer to. Or even a case where
> > if an async MPM exists, mod_reqtimeout shouldn’t be needed at all?
>
> Maybe it is not needed for the handshake any longer, but it allows fine
> grained timeouts on reading speed for headers and request
> bodies which I am not sure event could deal with. At least it cannot now.
It's not needed for the handshake *if* we have a HandshakeTimeout or
something (accept() is already done at this point so AcceptTimeout
would be a bit misleading), but currently using s->timeout is not
really the same.
So far this series is only about handshakes, but indeed if/when all
the request read phases can go async we probably want the fine grained
timeouts/rates from mod_reqtimeout to be available still (so either a
better interaction between the MPM and reqtimeout, or general
HeaderTimeout/BodyTimeout/... directives applicable by the MPM but
we'd still need something to count for bytes/rates in the different
states).
Regards;
Yann.
