Mailing List Archive

> Am 04.11.2021 um 13:14 schrieb Ruediger Pluem <rpluem@apache.org>:
>
>
>
> On 11/4/21 10:42 AM, icing@apache.org wrote:
>> Author: icing
>> Date: Thu Nov 4 09:42:45 2021
>> New Revision: 1894728
>>
>> URL: http://svn.apache.org/viewvc?rev=1894728&view=rev
>> Log:
>> * mod_http2: a regression in v1.15.24 of the modules was fixed that
>> could lead to httpd child processes not being terminated on a
>> graceful reload or when reaching MaxConnectionsPerChild.
>> When unprocessed h2 requests were queued at the time, these could stall.
>> See <https://github.com/icing/mod_h2/issues/212>.
>> [@hansborr, @famzah, Stefan Eissing]
>>
>>
>> Added:
>> httpd/httpd/trunk/changes-entries/h2_graceful_stall.txt
>> Modified:
>> httpd/httpd/trunk/modules/http2/h2_session.c
>> httpd/httpd/trunk/modules/http2/h2_workers.c
>>
>
>> Modified: httpd/httpd/trunk/modules/http2/h2_workers.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_workers.c?rev=1894728&r1=1894727&r2=1894728&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http2/h2_workers.c (original)
>> +++ httpd/httpd/trunk/modules/http2/h2_workers.c Thu Nov 4 09:42:45 2021
>> @@ -461,8 +461,6 @@ apr_status_t h2_workers_unregister(h2_wo
>> void h2_workers_graceful_shutdown(h2_workers *workers)
>> {
>> workers->shutdown = 1;
>> - workers->min_workers = 1;
>
> Isn't this needed to ensure that all idle workers are shutdown?

I decided to let the HMinWorkers live. There might be many request queued and still need processing.

> It is used in h2_fifo_term and wake_non_essential_workers.
>
>> workers->max_idle_duration = apr_time_from_sec(1);
>> - h2_fifo_term(workers->mplxs);
>
> Why is this no longer needed? Is wake_non_essential_workers below enough?

The h2_fifo_term was done, so that workers do no longer accept new requests. This
was the problem in <https://github.com/icing/mod_h2/issues/212>.

The story of the stalling was:
- h2 connection accepts request
- mpm shuts down gracefully
- h2 workers shut down, accept no more requests
- h2 connection tries to process the request it accepted
- no processing, no connection termination until Timeout

In the spirit that "graceful" means "process ongoing requests",
I decided that h2 workers need to stay available.

Kind Regards,
Stefan

>
>> wake_non_essential_workers(workers);
>> }
>>
>
> Regards
>
> Rüdiger
>

On 11/4/21 1:20 PM, stefan@eissing.org wrote:
>
>
>> Am 04.11.2021 um 13:14 schrieb Ruediger Pluem <rpluem@apache.org>:
>>
>>
>>
>> On 11/4/21 10:42 AM, icing@apache.org wrote:

>>> Modified: httpd/httpd/trunk/modules/http2/h2_workers.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_workers.c?rev=1894728&r1=1894727&r2=1894728&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/modules/http2/h2_workers.c (original)
>>> +++ httpd/httpd/trunk/modules/http2/h2_workers.c Thu Nov 4 09:42:45 2021
>>> @@ -461,8 +461,6 @@ apr_status_t h2_workers_unregister(h2_wo
>>> void h2_workers_graceful_shutdown(h2_workers *workers)
>>> {
>>> workers->shutdown = 1;
>>> - workers->min_workers = 1;
>>
>> Isn't this needed to ensure that all idle workers are shutdown?
>
> I decided to let the HMinWorkers live. There might be many request queued and still need processing.
>
>> It is used in h2_fifo_term and wake_non_essential_workers.
>>
>>> workers->max_idle_duration = apr_time_from_sec(1);
>>> - h2_fifo_term(workers->mplxs);
>>
>> Why is this no longer needed? Is wake_non_essential_workers below enough?
>
> The h2_fifo_term was done, so that workers do no longer accept new requests. This
> was the problem in <https://github.com/icing/mod_h2/issues/212>.
>
> The story of the stalling was:
> - h2 connection accepts request
> - mpm shuts down gracefully
> - h2 workers shut down, accept no more requests
> - h2 connection tries to process the request it accepted
> - no processing, no connection termination until Timeout
>
> In the spirit that "graceful" means "process ongoing requests",
> I decided that h2 workers need to stay available.

Thanks for explaining. You stop just accepting new stuff, but you no longer remove the capability to process what was already
accepted.

Regards

Rüdiger