Fix GH-18956: PHP-FPM Process Count Inconsistencies

[bukka]

This fixes incorrect decrement of the active number of processes. This was done in accepting stage before incrementing which is problematic if there are already some running processes as it decrements their number first and result in incorrect total (lower than the actual number).

In addition it also fixes GH-14212 as accept is done just once for keepalive connection so in this case the number of active connection just increasing with each request.

[bukka]

This needs more testing - I will go only for WST test as PHP test is too tricky for this and would not most likely work in the pipeline anyway.

[bukka]

So I have been testing and debugging this. At the end the best way to see what's going on was adding some extra logs which I have in this debug branch: master...bukka:php-src:fpm_gh18956_request_active_decrement_debug .

If it's done without keepalive, a single request call looks like following:

NOTICE: scoreboard active++
NOTICE: prev active: 0
NOTICE: new active: 1
NOTICE: fcgi reading 1 (len=8, type=1)
NOTICE: fcgi reading 2 (len=8, type=1, id=1)
NOTICE: req accepted: 0x5695f037fbc0
NOTICE: finished
NOTICE: req shutdown: 0x5695f037fbc0
NOTICE: scoreboard active--
NOTICE: prev active: 1
NOTICE: new active: 0
NOTICE: prev active: 0
NOTICE: new active: 0

But with keepalive (fastcgi_keep_conn on;), the situation is a bit different

NOTICE: scoreboard active++
NOTICE: prev active: 0
NOTICE: new active: 1
NOTICE: fcgi reading 1 (len=8, type=1)
NOTICE: fcgi reading 2 (len=8, type=1, id=1)
NOTICE: req accepted: 0x5695f037fd50
NOTICE: req shutdown: 0x5695f037fd50
NOTICE: scoreboard active--
NOTICE: prev active: 1
NOTICE: new active: 0
NOTICE: scoreboard active++
NOTICE: prev active: 0
NOTICE: new active: 1
NOTICE: fcgi reading 1 (len=0, type=5)
NOTICE: fcgi reading end 1
NOTICE: finished
NOTICE: prev active: 1
NOTICE: new active: 1

The problem is that there are two on_read calls because keepalive means that FPM (or more fastcgi.c) does second loop but if nginx gets single non keepalive http request, the second read is just 0 so FPM closes connection.

In this case it might seem logical to reduce active in fpm_request_finished (on_close hook) except it wouldn't work for non keepalive. Unfortunately there is currently no way to differentiate it in fpm_request_finished. Also on_close can be called in couple of other cases so it would need additional checking.

I will need to think about it more and see if there is some clean solution. Alternatively there might be some hacky solution to do that but it will require more checking as well. I will get this sorted - it will just require a bit of time to get right and properly test it.

[Appla]

Would it be possible to explicitly mark keepalive requests and reflect them in the statistics? For keepalive connections, subsequent reuse should only increment the reuse count without affecting the active/idle process state.

Based on this idea, I created a demonstration that minimizes changes to the existing API. ref