1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-19 12:00:55 +01:00

Add a specialized cache for storing "has setup ever worked?"

Summary:
Ref T11613. In D16503/T11598 I refined the setup flow to improve messaging for early-stage setup issues, but failed to fully untangle things.

We sometimes still try to access a cache which uses configuration before we build configuration, which causes an error.

Instead, store "are we in flight / has setup ever worked?" in a separate cache which doesn't use the cache namespace. This stops us from trying to read config before building config.

Test Plan:
Hit bad extension error with a fake extension, got a proper setup help page:

{F1812803}

Solved the error, reloaded, broke things again, got a "friendly" page:

{F1812805}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11613

Differential Revision: https://secure.phabricator.com/D16542
This commit is contained in:
epriestley 2016-09-12 07:31:53 -07:00
parent b822ceb6d5
commit 1ee426e4ac
3 changed files with 51 additions and 14 deletions

View file

@ -143,6 +143,39 @@ final class PhabricatorCaches extends Phobject {
} }
/* -( Server State Cache )------------------------------------------------- */
/**
* Highly specialized cache for storing server process state.
*
* We use this cache to track initial steps in the setup phase, before
* configuration is loaded.
*
* This cache does NOT use the cache namespace (it must be accessed before
* we build configuration), and is global across all instances on the host.
*
* @return PhutilKeyValueCacheStack Best available server state cache stack.
* @task setup
*/
public static function getServerStateCache() {
static $cache;
if (!$cache) {
$caches = self::buildSetupCaches('phabricator-server');
// NOTE: We are NOT adding a cache namespace here! This cache is shared
// across all instances on the host.
$caches = self::addProfilerToCaches($caches);
$cache = id(new PhutilKeyValueCacheStack())
->setCaches($caches);
}
return $cache;
}
/* -( Setup Cache )-------------------------------------------------------- */ /* -( Setup Cache )-------------------------------------------------------- */
@ -163,7 +196,7 @@ final class PhabricatorCaches extends Phobject {
public static function getSetupCache() { public static function getSetupCache() {
static $cache; static $cache;
if (!$cache) { if (!$cache) {
$caches = self::buildSetupCaches(); $caches = self::buildSetupCaches('phabricator-setup');
$cache = self::newStackFromCaches($caches); $cache = self::newStackFromCaches($caches);
} }
return $cache; return $cache;
@ -173,7 +206,7 @@ final class PhabricatorCaches extends Phobject {
/** /**
* @task setup * @task setup
*/ */
private static function buildSetupCaches() { private static function buildSetupCaches($cache_name) {
// If this is the CLI, just build a setup cache. // If this is the CLI, just build a setup cache.
if (php_sapi_name() == 'cli') { if (php_sapi_name() == 'cli') {
return array(); return array();
@ -188,7 +221,7 @@ final class PhabricatorCaches extends Phobject {
// If we don't have APC, build a poor approximation on disk. This is still // If we don't have APC, build a poor approximation on disk. This is still
// much better than nothing; some setup steps are quite slow. // much better than nothing; some setup steps are quite slow.
$disk_path = self::getSetupCacheDiskCachePath(); $disk_path = self::getSetupCacheDiskCachePath($cache_name);
if ($disk_path) { if ($disk_path) {
$disk = new PhutilOnDiskKeyValueCache(); $disk = new PhutilOnDiskKeyValueCache();
$disk->setCacheFile($disk_path); $disk->setCacheFile($disk_path);
@ -205,7 +238,7 @@ final class PhabricatorCaches extends Phobject {
/** /**
* @task setup * @task setup
*/ */
private static function getSetupCacheDiskCachePath() { private static function getSetupCacheDiskCachePath($name) {
// The difficulty here is in choosing a path which will change on server // The difficulty here is in choosing a path which will change on server
// restart (we MUST have this property), but as rarely as possible // restart (we MUST have this property), but as rarely as possible
// otherwise (we desire this property to give the cache the best hit rate // otherwise (we desire this property to give the cache the best hit rate
@ -230,7 +263,7 @@ final class PhabricatorCaches extends Phobject {
$tmp_dir = sys_get_temp_dir(); $tmp_dir = sys_get_temp_dir();
$tmp_path = $tmp_dir.DIRECTORY_SEPARATOR.'phabricator-setup'; $tmp_path = $tmp_dir.DIRECTORY_SEPARATOR.$name;
if (!file_exists($tmp_path)) { if (!file_exists($tmp_path)) {
@mkdir($tmp_path); @mkdir($tmp_path);
} }

View file

@ -74,6 +74,9 @@ abstract class PhabricatorSetupCheck extends Phobject {
$cache = PhabricatorCaches::getSetupCache(); $cache = PhabricatorCaches::getSetupCache();
$cache->setKey('phabricator.setup.issue-keys', $keys); $cache->setKey('phabricator.setup.issue-keys', $keys);
$server_cache = PhabricatorCaches::getServerStateCache();
$server_cache->setKey('phabricator.in-flight', 1);
if ($update_database) { if ($update_database) {
$db_cache = new PhabricatorKeyValueDatabaseCache(); $db_cache = new PhabricatorKeyValueDatabaseCache();
try { try {
@ -204,7 +207,8 @@ abstract class PhabricatorSetupCheck extends Phobject {
* @return bool True if we've made it through setup since the last restart. * @return bool True if we've made it through setup since the last restart.
*/ */
final public static function isInFlight() { final public static function isInFlight() {
return (self::getOpenSetupIssueKeys() !== null); $cache = PhabricatorCaches::getServerStateCache();
return (bool)$cache->getKey('phabricator.in-flight');
} }
final public static function loadAllChecks() { final public static function loadAllChecks() {

View file

@ -315,14 +315,6 @@ final class PhabricatorEnv extends Phobject {
* @task read * @task read
*/ */
public static function getEnvConfig($key) { public static function getEnvConfig($key) {
if (isset(self::$cache[$key])) {
return self::$cache[$key];
}
if (array_key_exists($key, self::$cache)) {
return self::$cache[$key];
}
if (!self::$sourceStack) { if (!self::$sourceStack) {
throw new Exception( throw new Exception(
pht( pht(
@ -331,6 +323,14 @@ final class PhabricatorEnv extends Phobject {
$key)); $key));
} }
if (isset(self::$cache[$key])) {
return self::$cache[$key];
}
if (array_key_exists($key, self::$cache)) {
return self::$cache[$key];
}
$result = self::$sourceStack->getKeys(array($key)); $result = self::$sourceStack->getKeys(array($key));
if (array_key_exists($key, $result)) { if (array_key_exists($key, $result)) {
self::$cache[$key] = $result[$key]; self::$cache[$key] = $result[$key];