Mailing List Archive

KeepAliveTimeout vs. event issue
IIUC event MPM can close keepalive connections up to 100ms than the
configured value.

If other software parses the Keep-Alive response header and fudges the
result by e.g. tens of milliseconds for its own TTL to avoid races,
should we internally try to avoid these whole-second
keepalivetimeouts? For example under event adding the 100ms back in
so at worst the timeout occurs in the same timeout=X advertised?

In the case I was working on, the client was doing no fudging on the
whole seconds returned by timeout=X so it's connection cache TTL can
easily get things wrong. A promising workaround is to configure e.g.
5999ms which will advertise timeout=5 but wait close to 6 seconds.

--
Eric Covener
covener@gmail.com
AW: KeepAliveTimeout vs. event issue [ In reply to ]
> -----Ursprüngliche Nachricht-----
> Von: Eric Covener <covener@gmail.com>
> Gesendet: Mittwoch, 19. Februar 2020 17:41
> An: Apache HTTP Server Development List <dev@httpd.apache.org>
> Betreff: KeepAliveTimeout vs. event issue
>
> IIUC event MPM can close keepalive connections up to 100ms than the
> configured value.
>
> If other software parses the Keep-Alive response header and fudges the
> result by e.g. tens of milliseconds for its own TTL to avoid races,
> should we internally try to avoid these whole-second
> keepalivetimeouts? For example under event adding the 100ms back in
> so at worst the timeout occurs in the same timeout=X advertised?

Just to be sure I get the issue correct. You say that with event a keepalive
connection might be kept open for x s + 100 ms if keepalivetimeout
is configured to x?
And the client thinks it is only kept open for x seconds as we announce
timeout=x in the keep-alive header?

I was not able to find any current RFC that describes the keep-alive
response header. I only found what Mozilla documents here:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive

And there they say that timeout is the *minimum* time the connection is
kept alive. So if we keep it open longer this would be fine.

Or is it exactly the other way around? Does event close it after
x s - 100 ms and hence earlier than announced?

Regards

Rüdiger
Re: AW: KeepAliveTimeout vs. event issue [ In reply to ]
On 2/20/20 8:36 AM, Pluem, Ruediger, Vodafone Group wrote:
>
>
>> -----Ursprüngliche Nachricht-----
>> Von: Eric Covener <covener@gmail.com>
>> Gesendet: Mittwoch, 19. Februar 2020 17:41
>> An: Apache HTTP Server Development List <dev@httpd.apache.org>
>> Betreff: KeepAliveTimeout vs. event issue
>>
>> IIUC event MPM can close keepalive connections up to 100ms than the
>> configured value.
>>
>> If other software parses the Keep-Alive response header and fudges the
>> result by e.g. tens of milliseconds for its own TTL to avoid races,
>> should we internally try to avoid these whole-second
>> keepalivetimeouts? For example under event adding the 100ms back in
>> so at worst the timeout occurs in the same timeout=X advertised?
>
> Just to be sure I get the issue correct. You say that with event a keepalive
> connection might be kept open for x s + 100 ms if keepalivetimeout
> is configured to x?
> And the client thinks it is only kept open for x seconds as we announce
> timeout=x in the keep-alive header?
>
> I was not able to find any current RFC that describes the keep-alive
> response header. I only found what Mozilla documents here:
>
> https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive
>
> And there they say that timeout is the *minimum* time the connection is
> kept alive. So if we keep it open longer this would be fine.
>
Same thing is written in this draft:
"A host MAY keep an idle connection open for longer than the time that it indicates, but it SHOULD attempt to retain a connection for at least as long as indicated."
https://tools.ietf.org/id/draft-thomson-hybi-http-timeout-01.html#keep-alive

Regards
Giovanni

> Or is it exactly the other way around? Does event close it after
> x s - 100 ms and hence earlier than announced?
>
> Regards
>
> Rüdiger
>
Re: KeepAliveTimeout vs. event issue [ In reply to ]
> Or is it exactly the other way around? Does event close it after
> x s - 100 ms and hence earlier than announced?

earlier then announced
AW: KeepAliveTimeout vs. event issue [ In reply to ]
> -----Ursprüngliche Nachricht-----
> Von: Eric Covener <covener@gmail.com>
> Gesendet: Donnerstag, 20. Februar 2020 12:33
> An: Apache HTTP Server Development List <dev@httpd.apache.org>
> Betreff: Re: KeepAliveTimeout vs. event issue
>
> > Or is it exactly the other way around? Does event close it after
> > x s - 100 ms and hence earlier than announced?
>
> earlier then announced

This is bad. Then we should add this back. If we keep it open a little bit longer (<= 1 s) it does not harm.

Regards

Rüdiger
Re: KeepAliveTimeout vs. event issue [ In reply to ]
On Thu, Feb 20, 2020 at 7:01 AM Pluem, Ruediger, Vodafone Group
<ruediger.pluem@vodafone.com> wrote:
>
>
>
> > -----Ursprüngliche Nachricht-----
> > Von: Eric Covener <covener@gmail.com>
> > Gesendet: Donnerstag, 20. Februar 2020 12:33
> > An: Apache HTTP Server Development List <dev@httpd.apache.org>
> > Betreff: Re: KeepAliveTimeout vs. event issue
> >
> > > Or is it exactly the other way around? Does event close it after
> > > x s - 100 ms and hence earlier than announced?
> >
> > earlier then announced
>
> This is bad. Then we should add this back. If we keep it open a little bit longer (<= 1 s) it does not harm.

I think it it as simple as:

Index: server/mpm/event/event.c
===================================================================
--- server/mpm/event/event.c (revision 1874247)
+++ server/mpm/event/event.c (working copy)
@@ -1218,7 +1218,7 @@
* timeout today. With a normal client, the socket will be readable in
* a few milliseconds anyway.
*/
- cs->queue_timestamp = apr_time_now();
+ cs->queue_timestamp = apr_time_now() + TIMEOUT_FUDGE_FACTOR;
notify_suspend(cs);

/* Add work to pollset. */


To compensate for the fudge factor here that pushes timeout_time ahead:

/* We process the timeout queues here only when their overall next
* expiry (read once above) is over. This happens accurately since
* adding to the queues (in workers) can only decrease this expiry,
* while latest ones are only taken into account here (in listener)
* during queues' processing, with the lock held. This works both
* with and without wake-ability.
*/
if (timeout_time && timeout_time < (now = apr_time_now())) {
timeout_time = now + TIMEOUT_FUDGE_FACTOR;

/* handle timed out sockets */
apr_thread_mutex_lock(timeout_mutex);

/* Processing all the queues below will recompute this. */
queues_next_expiry = 0;

/* Step 1: keepalive timeouts */
if (workers_were_busy || dying) {
process_keepalive_queue(0); /* kill'em all \m/ */
}
else {
process_keepalive_queue(timeout_time);
}
Re: KeepAliveTimeout vs. event issue [ In reply to ]
On Thu, Feb 20, 2020 at 7:11 AM Eric Covener <covener@gmail.com> wrote:
>
> On Thu, Feb 20, 2020 at 7:01 AM Pluem, Ruediger, Vodafone Group
> <ruediger.pluem@vodafone.com> wrote:
> >
> >
> >
> > > -----Ursprüngliche Nachricht-----
> > > Von: Eric Covener <covener@gmail.com>
> > > Gesendet: Donnerstag, 20. Februar 2020 12:33
> > > An: Apache HTTP Server Development List <dev@httpd.apache.org>
> > > Betreff: Re: KeepAliveTimeout vs. event issue
> > >
> > > > Or is it exactly the other way around? Does event close it after
> > > > x s - 100 ms and hence earlier than announced?
> > >
> > > earlier then announced
> >
> > This is bad. Then we should add this back. If we keep it open a little bit longer (<= 1 s) it does not harm.
>
> I think it it as simple as:
>
> Index: server/mpm/event/event.c
> ===================================================================
> --- server/mpm/event/event.c (revision 1874247)
> +++ server/mpm/event/event.c (working copy)
> @@ -1218,7 +1218,7 @@
> * timeout today. With a normal client, the socket will be readable in
> * a few milliseconds anyway.
> */
> - cs->queue_timestamp = apr_time_now();
> + cs->queue_timestamp = apr_time_now() + TIMEOUT_FUDGE_FACTOR;
> notify_suspend(cs);
>
> /* Add work to pollset. */

This has the unfortunate property that under low load you see the
extra 100ms every time. In the same env today you see it pretty much
exact.
Maybe Yann has something better.
Re: KeepAliveTimeout vs. event issue [ In reply to ]
On Thu, Feb 20, 2020 at 1:59 PM Eric Covener <covener@gmail.com> wrote:
>
> > Index: server/mpm/event/event.c
> > ===================================================================
> > --- server/mpm/event/event.c (revision 1874247)
> > +++ server/mpm/event/event.c (working copy)
> > @@ -1218,7 +1218,7 @@
> > * timeout today. With a normal client, the socket will be readable in
> > * a few milliseconds anyway.
> > */
> > - cs->queue_timestamp = apr_time_now();
> > + cs->queue_timestamp = apr_time_now() + TIMEOUT_FUDGE_FACTOR;
> > notify_suspend(cs);
> >
> > /* Add work to pollset. */
>
> This has the unfortunate property that under low load you see the
> extra 100ms every time. In the same env today you see it pretty much
> exact.
> Maybe Yann has something better.

Something like the attached patch maybe? i.e. limiting wakeups when
updating queues_next_expiry, yet using the current/real time when
processing/cleaning the queues (once woken up already).
Re: KeepAliveTimeout vs. event issue [ In reply to ]
> Something like the attached patch maybe? i.e. limiting wakeups when
> updating queues_next_expiry, yet using the current/real time when
> processing/cleaning the queues (once woken up already).

+1 thanks Yann.
Re: KeepAliveTimeout vs. event issue [ In reply to ]
On 02/20/2020 01:11 PM, Eric Covener wrote:
> On Thu, Feb 20, 2020 at 7:01 AM Pluem, Ruediger, Vodafone Group
> <ruediger.pluem@vodafone.com> wrote:
>>
>>
>>
>>> -----Ursprüngliche Nachricht-----
>>> Von: Eric Covener <covener@gmail.com>
>>> Gesendet: Donnerstag, 20. Februar 2020 12:33
>>> An: Apache HTTP Server Development List <dev@httpd.apache.org>
>>> Betreff: Re: KeepAliveTimeout vs. event issue
>>>
>>>> Or is it exactly the other way around? Does event close it after
>>>> x s - 100 ms and hence earlier than announced?
>>>
>>> earlier then announced
>>
>> This is bad. Then we should add this back. If we keep it open a little bit longer (<= 1 s) it does not harm.
>
> I think it it as simple as:
>
> Index: server/mpm/event/event.c
> ===================================================================
> --- server/mpm/event/event.c (revision 1874247)
> +++ server/mpm/event/event.c (working copy)
> @@ -1218,7 +1218,7 @@
> * timeout today. With a normal client, the socket will be readable in
> * a few milliseconds anyway.
> */
> - cs->queue_timestamp = apr_time_now();
> + cs->queue_timestamp = apr_time_now() + TIMEOUT_FUDGE_FACTOR;
> notify_suspend(cs);
>
> /* Add work to pollset. */
>

That looks like it could work. Have you tested? Any regressions?

Regards

Rüdiger

>