Changeset View
Standalone View
src/infrastructure/cluster/search/PhabricatorSearchCluster.php
- This file was added.
<?php | |||||
class PhabricatorSearchCluster | |||||
extends Phobject { | |||||
epriestley: This can be `final`, right?
This is enough of a pain that I probably wouldn't fix it here, but… | |||||
Not Done Inline Actions
Yeah, I believe so.
Naming things is hard. It wouldn't be too much of a pain to change it, if you want me to. 20after4: > This can be final, right?
Yeah, I believe so.
> This is enough of a pain that I probably… | |||||
const KEY_REFS = 'cluster.search.refs'; | |||||
protected $roles = array(); | |||||
protected $disabled; | |||||
protected $hosts = array(); | |||||
protected $hostsConfig; | |||||
protected $hostType; | |||||
protected $config; | |||||
const STATUS_OKAY = 'okay'; | |||||
const STATUS_FAIL = 'fail'; | |||||
public function __construct($host_type) { | |||||
$this->hostType = $host_type; | |||||
} | |||||
/** | |||||
* @throws Exception | |||||
*/ | |||||
public function newHost($config) { | |||||
$host = clone($this->hostType); | |||||
$host_config = $this->config + $config; | |||||
$host->setConfig($host_config); | |||||
$this->hosts[] = $host; | |||||
return $host; | |||||
} | |||||
Done Inline ActionsDouble setConfig()? epriestley: Double `setConfig()`? | |||||
Done Inline ActionsThis is intentional, to achieve hosts inheriting cluster-level settings. The first sets the defaults, then the second overrides them with host-specific values. Should I make this more explicit with separate setDefaultConfig and setHostConfig methods? Or at least explain it in a comment? 20after4: This is intentional, to achieve hosts inheriting cluster-level settings.
The first sets the… | |||||
Done Inline ActionsOh, just do something like this (this is conventional elsewhere): $full_config = $config + $this->config; $host->setConfig($full_config); Or rename setConfig() to addConfig() or something if you prefer. epriestley: Oh, just do something like this (this is conventional elsewhere):
```
$full_config = $config +… | |||||
public function getDisplayName() { | |||||
return $this->hostType->getDisplayName(); | |||||
} | |||||
public function setConfig($config) { | |||||
$this->config = $config; | |||||
if (!isset($config['hosts'])) { | |||||
$config['hosts'] = array( | |||||
array( | |||||
'host' => idx($config, 'host'), | |||||
'port' => idx($config, 'port'), | |||||
'protocol' => idx($config, 'protocol'), | |||||
'roles' => idx($config, 'roles'), | |||||
), | |||||
); | |||||
} | |||||
foreach ($config['hosts'] as $host) { | |||||
$this->newHost($host); | |||||
} | |||||
} | |||||
public function setDisabled($disabled) { | |||||
$this->disabled = $disabled; | |||||
return $this; | |||||
} | |||||
public function getDisabled() { | |||||
return $this->disabled; | |||||
} | |||||
public static function getConnectionStatusMap() { | |||||
return array( | |||||
self::STATUS_OKAY => array( | |||||
'icon' => 'fa-exchange', | |||||
'color' => 'green', | |||||
'label' => pht('Okay'), | |||||
), | |||||
self::STATUS_FAIL => array( | |||||
'icon' => 'fa-times', | |||||
'color' => 'red', | |||||
'label' => pht('Failed'), | |||||
), | |||||
); | |||||
} | |||||
public function isWritable() { | |||||
return $this->hasRole('write'); | |||||
} | |||||
public function isReadable() { | |||||
return $this->hasRole('read'); | |||||
} | |||||
public function hasRole($role) { | |||||
return isset($this->roles[$role]) && $this->roles[$role] === true; | |||||
} | |||||
public function setRoles(array $roles) { | |||||
foreach ($roles as $role => $val) { | |||||
$this->roles[$role] = $val; | |||||
} | |||||
return $this; | |||||
} | |||||
public function getRoles() { | |||||
return $this->roles; | |||||
} | |||||
public function getPort() { | |||||
return idx($this->config, 'port'); | |||||
} | |||||
public function getProtocol() { | |||||
return idx($this->config, 'protocol'); | |||||
} | |||||
public function getVersion() { | |||||
return idx($this->config, 'version'); | |||||
} | |||||
public function getHosts() { | |||||
return $this->hosts; | |||||
} | |||||
/** @return PhabricatorSearchHost */ | |||||
public function getAnyHostForRole($role) { | |||||
$hosts = $this->getAllHostsForRole($role); | |||||
if (empty($hosts)) { | |||||
throw new PhabricatorClusterNoHostForRoleException($role); | |||||
} | |||||
$random = array_rand($hosts); | |||||
return $hosts[$random]; | |||||
} | |||||
/** @return PhabricatorSearchHost[] */ | |||||
public function getAllHostsForRole($role) { | |||||
$hosts = array(); | |||||
foreach ($this->hosts as $host) { | |||||
if ($host->hasRole($role)) { | |||||
$hosts[] = $host; | |||||
} | |||||
} | |||||
return $hosts; | |||||
} | |||||
/** | |||||
* Get a reference to all configured fulltext search cluster services | |||||
* @return PhabricatorSearchCluster[] | |||||
*/ | |||||
public static function getAllServices() { | |||||
$cache = PhabricatorCaches::getRequestCache(); | |||||
$refs = $cache->getKey(self::KEY_REFS); | |||||
if (!$refs) { | |||||
$refs = self::newRefs(); | |||||
$cache->setKey(self::KEY_REFS, $refs); | |||||
} | |||||
return $refs; | |||||
} | |||||
/** find one random writable host from each service. | |||||
* @return PhabricatorSearchCluster[] writable cluster hosts | |||||
*/ | |||||
public static function getAllWritableHosts() { | |||||
$services = self::getAllServices(); | |||||
$all_writable = array(); | |||||
foreach ($services as $service) { | |||||
$all_writable += $service->getAllHostsForRole('write'); | |||||
} | |||||
return $all_writable; | |||||
} | |||||
public static function getValidHostTypes() { | |||||
return id(new PhutilClassMapQuery()) | |||||
->setAncestorClass('PhabricatorSearchHost') | |||||
->setUniqueMethod('getEngineIdentifier') | |||||
->execute(); | |||||
} | |||||
/** | |||||
* Create instances of PhabricatorSearchCluster based on configuration | |||||
* @return PhabricatorSearchCluster[] | |||||
*/ | |||||
public static function newRefs() { | |||||
$services = PhabricatorEnv::getEnvConfig('cluster.search'); | |||||
$types = self::getValidHostTypes(); | |||||
Not Done Inline ActionsUnconventional indent fomatting. epriestley: Unconventional indent fomatting. | |||||
$refs = array(); | |||||
foreach ($services as $config) { | |||||
if (!isset($types[$config['type']])) { | |||||
// this really should not happen as the value is validated by | |||||
// PhabricatorClusterSearchConfigOptionType | |||||
continue; | |||||
} | |||||
$type = $types[$config['type']]; | |||||
$cluster = new self($type); | |||||
$cluster->setConfig($config); | |||||
$refs[] = $cluster; | |||||
} | |||||
return $refs; | |||||
Done Inline ActionsIn this codebase, instead of "comment that this is impossible + continue", strongly prefer "throw". epriestley: In this codebase, instead of "comment that this is impossible + continue", strongly prefer… | |||||
Not Done Inline ActionsNoted. I'll fix that. 20after4: Noted. I'll fix that. | |||||
} | |||||
/** | |||||
* (re)index the document: attempt to pass the document to all writable | |||||
* fulltext search hosts | |||||
*/ | |||||
public static function reindexAbstractDocument( | |||||
PhabricatorSearchAbstractDocument $doc) { | |||||
$indexed = 0; | |||||
foreach (self::getAllWritableHosts() as $host) { | |||||
$host->getEngine()->reindexAbstractDocument($doc); | |||||
$indexed++; | |||||
} | |||||
if ($indexed == 0) { | |||||
throw new PhabricatorClusterNoHostForRoleException('write'); | |||||
} | |||||
} | |||||
/** | |||||
* Execute a full-text query and return a list of PHIDs of matching objects. | |||||
* @return string[] | |||||
*/ | |||||
public static function executeSearch(PhabricatorSavedQuery $query) { | |||||
$services = self::getAllServices(); | |||||
foreach ($services as $service) { | |||||
$hosts = $service->getAllHostsForRole('read'); | |||||
// try all hosts until one succeeds | |||||
foreach ($hosts as $host) { | |||||
$last_exception = null; | |||||
try { | |||||
$res = $host->getEngine()->executeSearch($query); | |||||
// return immediately if we get results without an exception | |||||
return $res; | |||||
} catch (Exception $ex) { | |||||
// try each server in turn, only throw if none succeed | |||||
$last_exception = $ex; | |||||
} | |||||
} | |||||
} | |||||
if ($last_exception) { | |||||
throw $last_exception; | |||||
} | |||||
return $res; | |||||
} | |||||
Not Done Inline ActionsThis is largely philosophical, but two other approaches you could take here:
epriestley: This is largely philosophical, but two other approaches you could take here:
- Catch all of… | |||||
Not Done Inline ActionsI like the idea of aggregate exception, though the relevant failure case is almost always going to be network related. Either something from CURL failing to make the connection, DNS failure, or some low level network error. In this case we should probably show the user an error along the lines of "Unable to reach any search services, try again later...", irrespective of which exception is encountered. 20after4: I like the idea of aggregate exception, though the relevant failure case is almost always going… | |||||
} |
This can be final, right?
This is enough of a pain that I probably wouldn't fix it here, but PhabricatorSearchService might be a more clear/consistent name for this class (e.g., the primary access method is ::getAllServices(), but as named it returns a list of "cluster" objects, not a list of "service" objects).