PHP5 session clean cron job causes OOM

[Steve Kamerman]

The php5 file-based session cleaner script /var/lib/php5/sessionclean is extremely expensive, taking over 10 seconds to run on my laptop, even when no PHP file-based session storage is enabled and the system is idle. My servers, on the other hand, are completely overwhelmed by this script, which runs every 30 minutes by default.

The script performs an lsof +d /var/lib/php5, which can itself take several minutes on my servers when they serving peak traffic of about 500 req/sec. This lsof command is extremely expensive (imagine iterating 5000 threads/procs, each of which has dozens or hundreds of open files). As a result of the load of this command, the server's connections back up leading to an out of memory condition (OOM) at which point the kernel tells oom_killer to start killing processes.

The really kicker here is that my session management is not done with files (most high-performance PHP sites would not be), so all this work is done in vain.

As a subject-matter expert in PHP and GNU/Linux, I recommend we update /usr/lib/php5/sessionclean and add /usr/lib/php5/sessionstorage
New versions: https://gist.github.com/kamermans/27218b1a6dd61756b647

The new sessionstorage script simply prints out which session.save_handlers are enabled in the installed PHP versions (not including the cli). The improved sessionclean script uses the sessionstorage to run only if the "files" session.save_handler is enabled, thus removing the lsof burden on most high-performance servers. Note that I am doing some creative scripting since Bash-style arrays are not available in Dash.

For reference, I am running on AWS c3.xlarge instances with NGINX, PHP5-FPM and Redis (no MySQL, no sessions at all). sessionclean kills my instances when the total CPU usage is around 20% (where 100% is a load of 4.0 on my 4-vCPU server), so I have a lot more room for requests if this is fixed.

ProblemType: Bug
DistroRelease: Ubuntu 14.04
Package: php5-common 5.5.9+dfsg-1ubuntu4.3
ProcVersionSignature: Ubuntu 3.13.0-29.53-generic 3.13.11.2
Uname: Linux 3.13.0-29-generic x86_64
ApportVersion: 2.14.1-0ubuntu3.3
Architecture: amd64
Date: Tue Aug 12 23:46:17 2014
Ec2AMI: ami-62a2730a
Ec2AMIManifest: (unknown)
Ec2AvailabilityZone: us-east-1c
Ec2InstanceType: c3.xlarge
Ec2Kernel: aki-919dcaf8
Ec2Ramdisk: unavailable
ProcEnviron:
 TERM=xterm
 PATH=(custom, no user)
 XDG_RUNTIME_DIR=<set>
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: php5
UpgradeStatus: Upgraded to trusty on 2014-07-30 (13 days ago)
modified.conffile..etc.cron.d.php5: [modified]
mtime.conffile..etc.cron.d.php5: 2014-08-12T20:02:50.878339

[Steve Kamerman (stevekamerman)]

On another note, why would we possibly need to run lsof every time we need to cleanup sessions? This is a very inefficient and expensive process. If someone knows how it came to be that this code was added, I would be very curious. Also, I see that recent versions of Ubuntu (14.04 for example) put a modules/ folder in /var/lib/php which is quite disconcerting since there are potentially thousands of session files in there as well.

[Ondřej Surý (ondrej)]

As an *expert* you surely heard of "rm /etc/cron.d/php5" or even "sed -i -e "s/^/#/" /etc/cron.d/php5"...

[Steve Kamerman (stevekamerman)]

Thanks for the completely non-constructive message. Google me if you want. Anyway, my point (and apparently your subpoint) is that the current script is not safe for production use. My improvements make it slightly more suitable out of the box.

[Ondřej Surý (ondrej)]

Should I be in awe or what? You can improve things even when not using the argument from authority.

My point is that the default cron job works and is perfectly safe for most deployments and it's documented in README.Debian that you need to tweak the session cleanup if you do modification to session handling. And I don't think we need to add more knobs for the edge cases.

[Steve Kamerman (stevekamerman)]

"You can improve things even when not using the argument from authority." I totally agree, I was just trying to make a point that I know what I'm talking about, as opposed to being a passive observer. You also know what you're talking about, and I can respect that as well.

If you seriously think running lsof on a server every 30 minutes regardless of the necessity is perfectly safe, then I guess we can agree to disagree :)

Prior to this bug report, my impression was that "Ubuntu is developed in an open fashion where everybody is welcome to contribute." (http://community.ubuntu.com/contribute/developers/) This incident, however, has put a bad taste in my mouth. Had I not been an open-source developer already, I would certainly have turned away from the open-source community.

Anyway, it is obvious that you are not going to consider my contribution, so I will stop pushing the issue.

[Ondřej Surý (ondrej)]

> I was just trying to make a point

You did make a point, but totally different from the one intended.

Also your proposed fix is incomplete...

# sed -n -e 's/^[[:space:]]*session.save_handler[[:space:]]*=[[:space:]]*\([[:alnum:]]\+\).*$/\1/p' /etc/php5/fpm/php.ini

# php5 -c /etc/php5/fpm/php.ini -d "error_reporting='~E_ALL'" -r 'echo ini_get("session.save_handler")."\n";'
files

The cron job is really meant for the default setup, since it also has other problems:

* The cron job doesn't handle custom session.save_path
* The cron job doesn't catch session files in subdirectories

> If you seriously think running lsof on a server every 30 minutes regardless of the necessity is perfectly safe, then I guess we can agree to disagree :)

If you can come up with a better way how to skip and not delete session files that are still used by long running scripts (e.g. https://bugs.debian.org/626640), then I would gladly accept the new solution.

I am also going to improve the wording of README.Debian to accent the need to modify the cron job if you modify the session handling.

[Ondřej Surý (ondrej)]

Try this updated script that now requires php5-cli to be installed:

http://anonscm.debian.org/cgit/pkg-php/php.git/tree/debian/sessionclean

[Robie Basak (racb)]

Steve, thank you for your report and for your time in helping to make Ubuntu better.

I'm not really sure how to respond here. Ondřej is the Debian maintainer for PHP, and Ubuntu's packaging is derived from Debian's packaging, so his opinion matters. I appreciate Ondřej's participation in Ubuntu's bug tracker for this reason.

I don't want Ubuntu's PHP packaging to deviate from Debian's. That's bad for everybody.

Could you two figure out the best way of handling this for all users, please, so that Debian can be updated (if needed) and I can follow any changes in Ubuntu?

[Daniel Holbach (dholbach)]

Thanks everyone for working on this.

[Steve Kamerman (stevekamerman)]

Thanks Ondřej, this is a much better script, and I agree that the lack of session.save_path support was rather annoying. This is still not optimal for high-performance servers, but it seems to be a good compromise for general use. Also, the fact that is honors SAPI-specific settings helps.

Also, thanks for posting the thread regarding active sessions over the gc_maxlifetime, that is a strange issue. It would probably be more efficient to iterate /proc/x/fd for each of the SAPIs, but this seems like it would break easily, plus some instances of the process (php, for example) might be indistinguishable from the CLI version. I'll take a crack at it this weekend for the sake of curiosity.

Good catch on the php-cli requirement - note that /usr/lib/php5/maxlifetime also uses php-cli in that way.

[Steve Kamerman (stevekamerman)]

What do you think about running this for each SAPI? It's actually *way* more efficient:

for pid in $(pidof "$sapi"); do
 find "/proc/$pid/fd" -lname "$session_save_path/*" -exec touch {} \;
done

That will iterate all the PIDs of the running SAPI (we'll need to iterate them as well), then find all the files it has open and touch them.

This is more efficient than lsof since it looks only at the files open by the SAPI in question, instead of every process. As an example, here's the timing of my proposed way and the current way:

kamermans@steve-ubuntu:~$ time for pid in $(pidof apache2); do sudo find "/proc/$pid/fd" -lname "/var/lib/php5/*" -print -exec touch {} \; ; done
/proc/4202/fd/21

real 0m0.143s
user 0m0.069s
sys 0m0.077s

kamermans@steve-ubuntu:~$ time sudo /usr/bin/lsof -w -l +d /var/lib/php5
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
apache2 4202 33 21uW REG 8,3 0 277620 /var/lib/php5/sess_bbn8sh6ljim484c4qt3inubhr3

real 0m1.079s
user 0m0.514s
sys 0m0.845s

Notice that my version returns /proc/4202/fd/21, but that is the file descriptor which is a symbolic link to /var/lib/php5/sess_bbn8sh6ljim484c4qt3inubhr3. Touching the link touches the file, so the effect is the same. Also, since the find command finds and touches the file in (almost) one operation, so it's unlikely the file will disappear between the time that it's found and the time that it's touched.

Yet another option is to alter the lsof call to restrict it to just processes that match the given name(s). This is also fast, although it not quite as fast as the aforementioned /proc method in my test:

kamermans@steve-ubuntu:~$ time sudo /usr/bin/lsof -w -l -a +d /var/lib/php5 -c apache2
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
apache2 4202 33 21uW REG 8,3 0 266585 /var/lib/php5/sess_18and1t1ej6frr9kjai3m1dbj4

real 0m0.182s
user 0m0.074s
sys 0m0.091s

Note that I added "-a" to AND the directory and command constraint, otherwise they would be OR'd.

[Steve Kamerman (stevekamerman)]

It just occurred to me that I was timing sudo in my last post, so here are the sudo-less numbers:

root@steve-ubuntu:~# time for pid in $(pidof apache2); do find "/proc/$pid/fd" -lname "/var/lib/php5/*" -print -exec touch {} \; ; done
/proc/30005/fd/21

real 0m0.042s
user 0m0.016s
sys 0m0.026s

root@steve-ubuntu:~# time /usr/bin/lsof -w -l +d /var/lib/php5
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
apache2 30005 33 21uW REG 8,3 0 268283 /var/lib/php5/sess_r5mhepbrhgmgsdnvkj72ppoc37

real 0m1.076s
user 0m0.517s
sys 0m0.846s

root@steve-ubuntu:~# time /usr/bin/lsof -w -l -a +d /var/lib/php5 -c apache2
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
apache2 30005 33 21uW REG 8,3 0 268283 /var/lib/php5/sess_r5mhepbrhgmgsdnvkj72ppoc37

real 0m0.070s
user 0m0.032s
sys 0m0.052s

[Steve Kamerman (stevekamerman)]

Ok, sorry for the repeated spamming tonight.

One problem with the solution you linked to is that it is extracting the gx_maxlifetime in seconds, then putting that in find's -cmin which expects minutes.

I've attached a script that incorporates the improvements of SAPI-specific session file cleaning, as well as my improved open-file touching code.

Here's the difference in time on one of my production machines:

# pidof php5-fpm | xargs php -r 'echo "$argc running threads\n";'
102 running threads

# time /usr/lib/php5/sessionclean /var/lib/php5 $(/usr/lib/php5/maxlifetime)
real 0m10.631s
user 0m7.876s
sys 0m2.738s

# time /tmp/sessionclean_proposed.sh
real 0m0.341s
user 0m0.126s
sys 0m0.241s

[Ondřej Surý (ondrej)]

> One problem with the solution you linked to is that it is extracting the gx_maxlifetime in seconds, then putting that in find's -cmin which expects minutes.

Good catch. The code was there, but it got deleted accidentally.

> What do you think about running this for each SAPI?

I am still not sure whether we are not breaking something with this change, but it's very good idea.

I have incorporated the idea, but kept the one run per directory and improved it that it just takes the highest gc_maxlifetime value. There's also an optimization to run pidof only once, since iterating through /proc/ on servers with many processes would also take it's toll (although I doubt the common setups include more than one SAPI on the server...)

Updated script is located in the git: http://anonscm.debian.org/cgit/pkg-php/php.git/tree/debian/sessionclean

Could you please proofread it and test it on your servers?

[Steve Kamerman (stevekamerman)]

Thanks, this is looking better now. Here are some notes:

1. PROC_NAMES should be defined as proc_names or used as PROC_NAMES (inconsistent case)
2. printf "%s:%s:%s\n" "$save_path" "$gc_maxlifetime" has 2 arguments but the definition takes 3, I would simplify it to this:
echo "$save_path:$gc_maxlifetime"
3. If apache2filter is installed, $proc_names will contains "apache2 apache2", which, when passed to pidof, will return all the PIDs twice, so I've uniq'd them by delimiting them with \n and using "sort -u | xargs pidof".
4. I'm not entirely sure how "cgi" method would work, but I suppose it means the php command is running on the system by itself, so I've used the following bit to discover that pidof requires "php", not "php5" since that is the real name of the process:
php -r 'sleep(60);echo "Done";' &
5. We've been using HHVM for a few years for one of our products, and I could easily add support for that ("hhvm --php -r"...), but things seem a bit out of order there, mainly, the php.ini is in /etc/hhvm instead of /etc/php5/hhvm

I've attached an updated script.

[Steve Kamerman (stevekamerman)]

I've added HHVM support and discovered another issue along the way: session.save_path can be empty and empty string. This is actually the default in stock PHP and HHVM. The PHP docs imply that an empty string is equal to "/tmp" (http://php.net/manual/en/session.configuration.php#ini.session.save-path), although that doesn't make much sense in Windows. Anyway, I am now checking for that as well, so if session.save_path=="", we use "/tmp".

[Steve Kamerman (stevekamerman)]

It seems likely that HHVM will find its way into the Debian repo, so I'm going to see if they will set the session.save_path to /var/lib/php5: https://github.com/hhvm/packaging/pull/67

[Steve Kamerman (stevekamerman)]

I have a feeling the CGI-mode process name is php5-cgi. I've installed it, but I haven't been able to get Apache 2.4 and mod_actions to play nice with it yet, so I haven't confirmed it.

[Steve Kamerman (stevekamerman)]

I noticed that you changed the default session.save_path to /var/lib/php5/sessions, which I think is a great idea (I have no idea what the modules/ dir is for).

In order to align the HHVM packaging with the Debian PHP packaging I have some questions:

1. Will PHP 5.6 be the default in Ubuntu 14.10?
2. Will these updated be present in 14.04.x?
3. Will these changes be present in PHP 5.5.x?
4. Are you going to be the Debian-side maintainer for the Debian-repo HHVM package?

The note to "see /usr/lib/php5/maxlifetime" is no longer needed as it doesn't exist
http://anonscm.debian.org/cgit/pkg-php/php.git/tree/debian/php5-common.php5.cron.d?h=master-5.6

I was able to get php-cgi working, and it uses the binary named php5 (assuming the user follows the example in "/etc/apache2/conf-available/serve-cgi-bin.conf"). Since the name php5 is ambiguous with /usr/bin/php5, we can pass the full path to the executable, "/usr/lib/cgi-bin/php5", to pidof.

This latest script builds on the last one, but is over 2x faster by avoiding multiple calls to PHP per SAPI. Instead, it uses foreach(ini_get_all("session")) to get all the session config options, then uses simple commands to parse config options out. It seems like this is a little overkill, but I think it makes the code a bit more readable, and it's a lot less expensive since PHP doesn't need to start up with all of its modules 3 times for each SAPI.

[Ondřej Surý (ondrej)]

> 1. PROC_NAMES should be defined as proc_names or used as PROC_NAMES (inconsistent case)

Good catch, fixed.

> 2. printf "%s:%s:%s\n" "$save_path" "$gc_maxlifetime" has 2 arguments but the definition takes 3, I would simplify it to this:
echo "$save_path:$gc_maxlifetime"

Good catch, fixed.

> 3. If apache2filter is installed, $proc_names will contains "apache2 apache2", which, when passed to pidof, will return all the PIDs twice, so I've uniq'd them by delimiting them with \n and using "sort -u | xargs pidof".

apache2 and apache2filter cannot be co-installed.

> 4. I'm not entirely sure how "cgi" method would work, but I suppose it means the php command is running on the system by itself, so I've used the following bit to discover that pidof requires "php", not "php5" since that is the real name of the process:
php -r 'sleep(60);echo "Done";' &

The default package config points to /usr/lib/cgi-bin/php5

> 5. We've been using HHVM for a few years for one of our products, and I could easily add support for that ("hhvm --php -r"...), but things seem a bit out of order there, mainly, the php.ini is in /etc/hhvm instead of /etc/php5/hhvm

It's up to the HHVM packages to sort this out. I doubt they will depend on php5-common at all. So I am skipping changes related to HHVM. It's start to feel like bikesheding.

> 1. Will PHP 5.6 be the default in Ubuntu 14.10?

Nope, there was a separate bug report about this, and my recommendation was to stay with 5.5.<latest>

> 2. Will these updated be present in 14.04.x?
> 3. Will these changes be present in PHP 5.5.x?

I will merge these changes to master-5.5 branch in Debian git repo (and my PPA ppa:ondrej/php5). It's up to Ubuntu maintainer (@racb) to pick that or not.

> 4. Are you going to be the Debian-side maintainer for the Debian-repo HHVM package?

Nope, there are some Debian folks from fcbk who wanted to do the work.

> Since the name php5 is ambiguous with /usr/bin/php5, we can pass the full path to the executable, "/usr/lib/cgi-bin/php5", to pidof.

There's no need to do so. The worst case scenario is when /usr/bin/php5 session would get touched as well.

> it's a lot less expensive since PHP doesn't need to start up with all of its modules 3 times for each SAPI.

Again it feels like over-optimizing, but I have merged a variant of this.

> session.save_path can be empty and empty string.

It cannot be an empty string in Debian, since we patch the sources to have a default to point to /var/lib/php5/sessions/

Ondrej

[Steve Kamerman (stevekamerman)]

Indeed, I think we've passed the 80/20 mark on this one. I'll test your updated one today and we should be good.

[Steve Kamerman (stevekamerman)]

Issue on line 8:
  echo -e "$SAPIS" | \

The "-e" is ending up in the output (perhaps dash's echo has no "-e") resulting in this error:
  ./sessionclean: 10: [: /etc/php5/-e: unexpected operator
I've removed the -e as our input is clean anyway.

Also, I've run this to normalize the spacing:
  sed -i "s/\t/ /" sessionclean

Other than that everything is working well. Thanks for the help here!

[Ondřej Surý (ondrej)]

> Issue on line 8:

Missed this in all the cruft generated by "sh -x", and since the other output looked ok...

Thanks for noticing that...

Also thank you for the feedback, this was very valuable after all.

I probably owe you an apology, so please accept my apology I should have been more calm from the beginning.

I have probably met too many clueless people who have waved with "expertness" around and I am very glad this wasn't the case and this was very productive and expert interaction in the end.

[Steve Kamerman (stevekamerman)]

Thanks Ondřej, no apology needed, this was indeed a constructive thread. Sometimes us open source geeks get all too used to being in charge and we forget how to communicate effectively. If I'm up in your neck of the woods some time I'll look you up and we can grab a beer :)

Also, I'll do a PR on Facebook's HHVM repo which includes an HHVM-specific GC script (basically a non-iterative version of this one).

[Ondřej Surý (ondrej)]

There's one more improvement the packages could do:

Each SAPI could drop it's session snippet to /var/lib/php5/sapi/<sapi> and the sessionclean script would read those snippets and clean the directory according to really installed snippets.

That would make the sessionclean script SAPI agnostic and could support HHVM out of the box...

So if you talk to HHVM packagers and they show interest, I could implement this.

[Steve Kamerman (stevekamerman)]

That does seem more appropriate than putting all the SAPI methods in php-common. The HHVM guys (Paul Tarjan, etc) gave me the green light to do the session cleanup bit, so I'll take care of it once we settle on a script.

Wouldn't /usr/lib/php5/sapi/<sapi> be a better location for scripts?

[Ondřej Surý (ondrej)]

> Wouldn't /usr/lib/php5/sapi/<sapi> be a better location for scripts?

Yup...

[Ondřej Surý (ondrej)]

Either

```
for sapi in /usr/lib/php5/sapi/*; do
  . $sapi
```

and have the /usr/lib/php5/sapi/<sapi> contain shell snippet with definitions

or

```
cat /usr/lib/php5/sapi/* | while IFS=: read -r VAR1 VAR2 VAR3 VAR4; do
```

and keep the current format...

[Steve Kamerman (stevekamerman)]

I prefer option #1 - sourcing the sessionclean scripts from /usr/lib/php5/sapi/* or perhaps /usr/lib/php5/sapi/<sapi>/sessionclean so we can make room for more sapi-specific scripts should the need arise.

If we keep the logic in php-common, a con is that we can't use it for HHVM (not that HHVM should drive the decision).
A pro for this method is that we can have /usr/lib/php5/sapi/<sapi>/sessionconfig scripts that simply spit out the params we need, then we feed them into the sessionclean script as you've described (sort of like /usr/lib/php5/maxlifetime is used now)
Another pro for this method is that we can optimize cleanup by de-duplicating the session.save_paths and using max(session.gc_maxlifetime).

[Steve Kamerman (stevekamerman)]

Btw, if we go the route of separate sapi folders inside /usr/lib/php5/sapi/ we should also move php5-fpm-checkconf into /usr/lib/php5/sapi/fpm/ (I used "fpm" instead of "php5-fpm" here since that how it is in /etc/php5/).

[Robie Basak (racb)]

Sorry for the late reply. I've been out for the last couple of weeks.

On Tue, Aug 19, 2014 at 08:13:55AM -0000, Ondřej Surý wrote:
> > 2. Will these updated be present in 14.04.x?
> > 3. Will these changes be present in PHP 5.5.x?
>
> I will merge these changes to master-5.5 branch in Debian git repo (and
> my PPA ppa:ondrej/php5). It's up to Ubuntu maintainer (@racb) to pick
> that or not.

For updates to 14.04, I have to follow our SRU process for any updates,
and can only push fixes for high-impact bugs with low regression risk.
Each update is a separate effort and much more involved than a simple
push. It isn't clear to me that this change qualifies, but I'm open to
discussing this. The rationale, requirements and process is described
here: https://wiki.ubuntu.com/StableReleaseUpdates

By PHP 5.5.x, do you mean 14.10? We're past feature freeze for 14.10
now, but I can push bugfixes that aren't new features. The criteria
isn't much different, but I am permitted to judge that for myself and
can just upload. Again, I'm open to discussion and am happy to upload
fixes that are justified.

For anything else, we'll have to wait for the next release cycle.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[RickB (rick-777)]

My 14.10 laptop just froze at the moment when the cron.d job kicked in (9 minutes past the hour). There was not much else running at the time.

[Sezer Yalcin (sy-y)]

I just noticed 800+ processes on my raspberry pi 2 ubuntu 14.04, all were spawns from this cron.

Although cron script is bad in terms of cpu use, I see it's set to run every 30 minutes. So I am also thinking something else is also going on, probably some hanging with lsof anyways.

This script can be run once a day (not 48 times a day as currently). Session information is stored in /tmp so if you don't get it full, you may even disable it completely, especially if you don't use php much.

[Robie Basak (racb)]

AFAICT, a fix to this has not yet been committed to Debian VCS so I have nothing to track or cherry-pick in Ubuntu yet. Is this accurate or have I missed something?

[Ondřej Surý (ondrej)]

@racb did you check the master-5.6 branch? I am quite sure that Debian jessie already has the new sessionclean script.

[Robie Basak (racb)]

Ah sorry, thanks. I looked again. I saw the commits but assumed that 2014-08 was too old, but the comments do actually date from them.

So I think that Wily contains the new script, but Trusty will need it cherry-picked. Setting bug tasks accordingly.

[Jeff Anttila (jeff-anttila)]

Any ETA on a port to Debian-ARM?

Currently using Linux version 4.1.15+ (dc4@dc4-XPS13-9333) (gcc version 4.8.3 20140303 (prerelease) (crosstool-NG linaro-1.13.1+bzr2650 - Linaro GCC 2014.03) ) #830 Tue Dec 15 16:58:28 GMT 2015

Thanks.

Jeff Anttila

[Kaijia Feng (fengkaijia)]

Any fix to 14.04 yet? This bug is still causing me to manually reboot my server every week otherwise it would be too many processes to process any requests.

[Ondřej Surý (ondrej)]

You might try the script I use in ppa:ondrej/php: http://anonscm.debian.org/cgit/pkg-php/php.git/tree/debian/sessionclean?h=master-5.5

[Nish Aravamudan (nacc)]

Hello all, I have provided a testbuild for this bug at: https://launchpad.net/~nacc/+archive/ubuntu/lp1356113. Please test this version of the script (sessionclean) and report back results.