Hi Philipp,
OK I now found a moment to spare some time on your patch. During my
first lecture I didn't understand that it relied on SIGUSR2 to
aggregate counters. I'm seeing several issues with that approach :
- the time to scan all the proxies can be huge. Some people on this
list run with more than 50000 backends. And the processing is
serialized in shm_proxy_update() so that means that even if they
run 32 processes on 32 cores, they'll have to wait 32 times the
time it takes to process 50000 backends.
- stats cannot easily be summed this way. The max are never valid
this way, because max(A+B) is somwhere between max(A,B) and
max(max(A)+max(B)). For this reason it's important to update stats
in real time using atomic operations.
- the use of semaphores should *really* be avoided, as they create
huge trouble in production because they last after the process'
death. Here for example if you send a SIGUSR2 to your processes,
find that they're taking too much time to aggregate and decide
to kill the culprit, the other ones will be stuck forever and
when you decide to kill and restart the service, the old IPC is
still there until you reach the moment where there are no more
left and admins reboot the system because nowadays nobody thinks
to run "ipcs -a" then "ipcrm". I'd rather welcome a solution
involving atomic operations and/or mutexes even if it's limited
to a few architectures/operating systems. Note that the shared_ctx
used for SSL actually does that.
- you have a process that remains stuck for up to 100ms per process
in the update_shm_proxy() function! By this time it's not processing
any traffic and is waiting for nothing, you must never ever do such
a thing. Imagine if someone sends a signal even just once a minute
to collect stats, you'll have big holes in your traffic graphs!
What I'd like to have instead would be a per-proxy shared memory segment
for stats in addition to the per-process one, that is updated using
atomic operations each time other stats are updated. The max are a bit
tricky as you need to use a compare-and-swap operation but that's no big
deal. Please note that before doing this, it would be wise to move all
existing stats to a few dedicated structures so that in each proxy, server,
listener and so on we could simply have something like this :
struct proxy_stats *local;
struct proxy_stats *global;
As you guessed it local would be allocated in per process while global
would be shared between all of them.
Another benefit would be that we could improve the current sample fetch
functions which already look at some stats and use the global ones. That's
even more important for maxconn where it would *open* the possibility to
monitor the global connection count and not just the per-process one (but
there are other things to do prior to this being possible, such as
inter-process calls). However without inter-process calls we could decide
that we can slightly overbook the queue by up to one connection max per
process and that could be reasonably acceptable while waiting for a longterm
multi-threaded approach though.
Best regards,
Willy
