Add cgroups support to PHP-FPM pools

[rpv-tomsk]

This patch allows to configure cgroup where to put php-fpm pool worker processes, configured per pool separately.

Moving workers into cgroup by external applications is too inaccurate and unuseable in heavily-loaded envirnonments. Applying cgroup to worker at its start is correct solution for this, and can be widely used to pool resource control and limiting.

We use this solution since 5.6, with 7.0 too. But these branches are already closed for new features, so I made PR against 7.1.

Many Thanks to PHP Team.

[bukka]

I'm not really sure if using libcg is a good idea considering that there are just 2 fixes in 2015 and all other development ends in 2014. So basically that library seems dead to me.

What would be much better to have is our own pluggable interface that would support just the needed operations in drivers. Then we could have one driver for fs (preferably using v2 api) and one for systemd (yes something like what libcontainer does but on much smaller case to cover only the needed functionality).

[rpv-tomsk]

I'm not really sure if using libcg is a good idea considering that there are just 2 fixes in 2015 and all other development ends in 2014. So basically that library seems dead to me.

As for me, an example of dead library is libdbi (https://sourceforge.net/projects/libdbi/files/) which has huge memleaks without fixes for years.

Libcgroup (libcg library + tools) just works and it does all it should to do.
Looking into Https://sourceforge.net/projects/libcg/files/libcgroup/ - it has 5000 downloads per week only for source code, and it is presented in the distribution packages too.
So, it is used widely enough and can't be called dead. IMHO.

Yes, I'm agreed what systemd used widely for cgroups now. But I see no other way (without using this patch) how to separate pool processes into appropriate cgroup without starting many php-fpm instances (with dedicated fpm instance for each pool).

What would be much better to have is our own pluggable interface

That will be more flexible, agreed.
But we just shared what we use ourselves, in hope that it will be useful for others too.

Then we could have one driver for fs (preferably using v2 api)

What do you mean by 'fs'? Please point me.

and one for systemd (yes something like what libcontainer does
but on much smaller case to cover only the needed functionality).

How that can work / be configured for setting cgroup per pool?
What is the intended scenario?

[bukka]

Well I think that's ok if you use it for your work but adding that to the upstream project is a different thing IMHO. We should really look if there is any work on the library before we add a dependency on it.

By fs I mean doing operation on file system kernel API as documented in here https://www.kernel.org/doc/Documentation/cgroup-v2.txt . The libcg is just a wrapper around that (it's actually using v1 I think but in general it's just a wrapper around fs operations)

I don't really know much about systemd and if it would be really useful for anything (even in the future). I just know that it has its own cgroup interface (which actually wraps fs api) and it is used libcontainer for some operations ( see https://github.com/docker/libcontainer/blob/master/cgroups/systemd/apply_systemd.go ) that also wraps fs api but has some systemd specific things in it. However that's not something that I'm suggesting to do. What I mean is that the API should be pluggable and the specific operation abstracted. But maybe that's something that could be added later.

The main thing that I would prefer is having that done on the kernel API (preferably v2) rather then using libcg.

[rpv-tomsk]

Well I think that's ok if you use it for your work but adding that to the upstream project is a different thing IMHO.

Yes, I know about this difference, and after detailed analysis I agreed with you - adding support of (an become outdated) v1 cgroups is not an option. Our patch slightly outdated, I'm too late with publishing it :-)

Thanks for your explanations.

We will continue to use v1, but will try to start preparations for cgroups v2 as part of public work.

Let's discuss future steps on this issue task.

What I mean is that the API should be pluggable and the specific operation abstracted.
But maybe that's something that could be added later.

New option should be added into 'global' section to point which library to load, right?
Is that library should be designed for one 'cgroups' task only?
Should it be possible to load some libraries at once?
Should loaded library declare pool configuration options it handles or it may be enough to have single 'cgroup' option static support?

Have you looked into sources to build your own draft plan how that can be implemented?

Your support and mentoring is very welcome.
Thanks.

[bukka]

Ah sorry I didn't actually mean pluggable in terms of having a dynamic loading of libs. I thought more about a generic API that can allow additional implementation. Basically in a similar way as it is done for fpm_event but just with the callbacks needed for cgroup operations (e.g. checking existence of cgroup, ataching current process into it and so on).

In terms of global option it would certainly make sense to add an option (e.g. cgroup_driver) which would select an implementation and have some default if not set. However if there is just one driver, that option can be added later if another driver is added. Then we would just need a pool based option to select the cgroup as it's already in this PR.

To sum it up the whole thing is mainly about abstracting the operation to the generic struct with callbacks for all needed operations and have one implementation that will be selected by default.

[krakjoe]

So it looks like we are more interested in a kernel api version of this patch using the most recent API ... since the conversation kind of stopped there, I'm going to assume this work is abandoned and close the PR.

[rpv-tomsk]

I completely agree that 'kernel api is preferred', but for now I have no other patch proposals.

When I will have, I will open new PR.

Thanks.