Welcome! Log In Create A New Profile

Advanced

Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)

Maxim Dounin
January 09, 2024 01:00AM
Hello!

On Mon, Jan 08, 2024 at 01:31:11PM +0000, J Carter wrote:

> On Mon, 8 Jan 2024 11:25:55 +0000
> J Carter <jordanc.carter@outlook.com> wrote:
>
> > Hello,
> >
> > On Mon, 27 Nov 2023 05:50:27 +0300
> > Maxim Dounin <mdounin@mdounin.ru> wrote:
> >
> > > # HG changeset patch
> > > # User Maxim Dounin <mdounin@mdounin.ru>
> > > # Date 1701050170 -10800
> > > # Mon Nov 27 04:56:10 2023 +0300
> > > # Node ID 00c3e7333145ddb5ea0eeaaa66b3d9c26973c9c2
> > > # Parent 61d08e4cf97cc073200ec32fc6ada9a2d48ffe51
> > > AIO operations now add timers (ticket #2162).
> > >
> > > Each AIO (thread IO) operation being run is now accompanied with 1-minute
> > > timer. This timer prevents unexpected shutdown of the worker process while
> > > an AIO operation is running, and logs an alert if the operation is running
> > > for too long.
> >
> > Shouldn't this timer's duration be set to match worker_shutdown_timeout's
> > duration rather than being hard coded to 60s ?
>
> Ah nevermind, I understand.
>
> These timers will either expire from passing the 60s set duration, or
> will expire as worker_process_timeout itself expires, kills the
> connection and times out associated timers (including the aio timers).
>
> Setting it to worker_shutdown_timeout's duration would be pointless
> (an 'infinite' timer would give the same result).
>
> So the only situation in which a different value for these AIO
> timers would make sense is if these AIO operations are expected to
> take longer 60s, but less than worker_shutdown_timeout (in cases
> where it has been increased from it's own default of 60s).
>
> In that case the AIO operation's timeout would have to be one
> (or more) of it's own directives, with a value less than
> worker_shutdown_timeout.

Not really.

When worker_shutdown_timeout expires, it tries to terminate the
request, but it can't as long as an AIO operation is running.
When the AIO operation completes, the request will be actually
terminated and the worker process will be allowed to exit. So far
so good.

But if the AIO operation never completes, the timer will expire
after 1 minute, will log an alert, and the worker processes will
be anyway allowed to exit (with the request still present). This
might not be actually possible though - for example,
ngx_thread_pool_exit_worker() will just block waiting for all
pending operations to complete.

In theory, the situation when an AIO operation never completes
should never happen, and just a counter of pending AIO
operations can be used instead to delay shutdown (which is
essentially equivalent to an infinite timer).

In practice, though, using a large enough guard timer might be
better: it provides additional level of protection against bugs or
system misbehaviour, and at least logs an alert if something
really weird happens. It is also looks more universal and in line
with current approach of using existing timers as an indicator
that something is going on and shutdown should be delayed.

The timer is expected to be "large enough", since we cannot do
anything meaningful with an AIO operation which never completes,
we can only complain loudly, so the timer should never expire
unless there is something really wrong. This is not the case with
worker_shutdown_timeout: it can be set to an arbitrary low value,
which is not expected to mean that something is really wrong if
the timer expires, but rather means that nginx shouldn't try to
process remaining requests, but should instead close them as long
as it can do so. That is, worker_shutdown_timeout does not fit
semantically. Further, worker_shutdown_timeout is not set by
default, so it simply cannot be used.

The 1 minute was chosen as it matches default send_timeout, which
typically accompanies AIO operations when sending responses (and
also delays shutdown, so no "open socket left" alerts are normally
seen). Still, send_timeout is also quite different semantically,
and therefore a hardcoded value is used instead.

I don't think there are valid cases when AIO operations can take
longer than 1 minute, as these are considered to be small and fast
operations, which are normally done synchronously within nginx
event loop when not using AIO, and an operations which takes 1m
would mean nginx is completely unresponsive. Still, if we'll
found out that 1 minute is not large enough in some cases,
we can just bump it to a larger value.

[...]

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH 0 of 4] aio fixes

Maxim Dounin 405 November 26, 2023 10:10PM

[PATCH 3 of 4] Silenced complaints about socket leaks on forced termination

Maxim Dounin 61 November 26, 2023 10:10PM

Re: [PATCH 3 of 4] Silenced complaints about socket leaks on forced termination

Sergey Kandaurov 48 January 26, 2024 07:28AM

Re: [PATCH 3 of 4] Silenced complaints about socket leaks on forced termination

Maxim Dounin 45 January 29, 2024 02:32AM

[PATCH 2 of 4] Upstream: fixed usage of closed sockets with filter finalization

Maxim Dounin 72 November 26, 2023 10:10PM

Re: [PATCH 2 of 4] Upstream: fixed usage of closed sockets with filter finalization

Sergey Kandaurov 44 January 26, 2024 07:28AM

Re: [PATCH 2 of 4] Upstream: fixed usage of closed sockets with filter finalization

Maxim Dounin 44 January 29, 2024 01:44AM

Re: [PATCH 2 of 4] Upstream: fixed usage of closed sockets with filter finalization

Sergey Kandaurov 43 January 29, 2024 08:24AM

Re: [PATCH 2 of 4] Upstream: fixed usage of closed sockets with filter finalization

Maxim Dounin 44 January 29, 2024 10:08PM

[PATCH 4 of 4] AIO operations now add timers (ticket #2162)

Maxim Dounin 61 November 26, 2023 10:10PM

Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)

J Carter 43 January 08, 2024 06:28AM

Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)

J Carter 47 January 08, 2024 08:32AM

Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)

Maxim Dounin 52 January 09, 2024 01:00AM

Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)

J Carter 62 January 09, 2024 10:12AM

Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)

Sergey Kandaurov 81 January 26, 2024 07:28AM

Re: [PATCH 4 of 4] AIO operations now add timers (ticket #2162)

Maxim Dounin 46 January 29, 2024 02:32AM

[PATCH 1 of 4] Fixed request termination with AIO and subrequests (ticket #2555)

Maxim Dounin 77 November 26, 2023 10:10PM

Re: [PATCH 1 of 4] Fixed request termination with AIO and subrequests (ticket #2555)

Roman Arutyunyan 40 January 26, 2024 07:04AM

Re: [PATCH 1 of 4] Fixed request termination with AIO and subrequests (ticket #2555)

Maxim Dounin 50 January 29, 2024 03:00AM

Re: [PATCH 1 of 4] Fixed request termination with AIO and subrequests (ticket #2555)

Roman Arutyunyan 56 January 29, 2024 09:10AM

Re: [PATCH 1 of 4] Fixed request termination with AIO and subrequests (ticket #2555)

Maxim Dounin 61 January 29, 2024 10:08PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 336
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready