1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 01:08:50 +02:00

Make disk-based setup caches more correct (but slower)

Summary:
Fixes T9599. When APC/APCu are not available, we fall back to a disk-based cache.

We try to share this cache across webserver processes like APC/APCu would be shared in order to improve performance, but are just kind of guessing how to coordinate it. From T9599, it sounds like we don't always get this right in every configuration.

Since this is complicated and error prone, just stop trying to do this. This cache has bad performance anyway (no production install should be using it), and we have much better APC/APCu setup instructions now than we did when I wrote this. Just using the PID is simpler and more correct.

Test Plan:
  - Artificially disabled APC.
  - Reloaded the page, saw all the setup stuff run.
  - Reloaded the page, saw no setup stuff run (i.e., cache was hit).
  - Restarted the webserver.
  - Reloaded the page, saw all the setup stuff run.
  - Reloaded again, got a cache hit.

I don't really know how to reproduce the exact problem with the parent PID not working, but from T9599 it sounds like this fixed the issue and from my test plan we still appear to get correct behavior in the standard/common case.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9599

Differential Revision: https://secure.phabricator.com/D14302
This commit is contained in:
epriestley 2015-10-19 11:14:46 -07:00
parent 057d62d570
commit d784bf1ea8
2 changed files with 7 additions and 33 deletions

View file

@ -206,40 +206,12 @@ final class PhabricatorCaches extends Phobject {
// otherwise (we desire this property to give the cache the best hit rate
// we can).
// In some setups, the parent PID is more stable and longer-lived that the
// PID (e.g., under apache, our PID will be a worker while the ppid will
// be the main httpd process). If we're confident we're running under such
// a setup, we can try to use the PPID as the basis for our cache instead
// of our own PID.
$use_ppid = false;
switch (php_sapi_name()) {
case 'cli-server':
// This is the PHP5.4+ built-in webserver. We should use the pid
// (the server), not the ppid (probably a shell or something).
$use_ppid = false;
break;
case 'fpm-fcgi':
// We should be safe to use PPID here.
$use_ppid = true;
break;
case 'apache2handler':
// We're definitely safe to use the PPID.
$use_ppid = true;
break;
}
// Unfortunately, we don't have a very good strategy for minimizing the
// churn rate of the cache. We previously tried to use the parent process
// PID in some cases, but this was not reliable. See T9599 for one case of
// this.
$pid_basis = getmypid();
if ($use_ppid) {
if (function_exists('posix_getppid')) {
$parent_pid = posix_getppid();
// On most systems, normal processes can never have PIDs lower than 100,
// so something likely went wrong if we we get one of these.
if ($parent_pid > 100) {
$pid_basis = $parent_pid;
}
}
}
// If possible, we also want to know when the process launched, so we can
// drop the cache if a process restarts but gets the same PID an earlier

View file

@ -63,7 +63,9 @@ final class PhabricatorDataCacheSpec extends PhabricatorCacheSpec {
private function initNoneSpec() {
if (version_compare(phpversion(), '5.5', '>=')) {
$message = pht(
'Installing the "APCu" PHP extension will improve performance.');
'Installing the "APCu" PHP extension will improve performance. '.
'This extension is strongly recommended. Without it, Phabricator '.
'must rely on a very inefficient disk-based cache.');
$this
->newIssue('extension.apcu')