Differential D17384 Diff 42039 src/infrastructure/cluster/search/PhabricatorSearchCluster.php

Changeset View

Standalone View

src/infrastructure/cluster/search/PhabricatorSearchCluster.php

This file was added.

				<?php

				class PhabricatorSearchCluster
				extends Phobject {
				epriestleyUnsubmitted Not Done Inline Actions 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). epriestley: This can be `final`, right? This is enough of a pain that I probably wouldn't fix it here, but…
				20after4AuthorUnsubmitted Not Done Inline Actions This can be final, right? Yeah, I believe so. 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 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';
				const KEY_HEALTH = 'cluster.search.health';

				protected $healthRecord;
				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->setConfig($this->config)
				->setConfig($config);
				$this->hosts[] = $host;
				epriestleyUnsubmitted Done Inline Actions Double `setConfig()`? epriestley: Double `setConfig()`?
				20after4AuthorUnsubmitted Done Inline Actions This 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…
				epriestleyUnsubmitted Done Inline Actions Oh, 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 +…
				return $host;
				}

				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();
				$refs = array();
				epriestleyUnsubmitted Not Done Inline Actions Unconventional indent fomatting. epriestley: Unconventional indent fomatting.

				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;
				}
				epriestleyUnsubmitted Done Inline Actions In 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…
				20after4AuthorUnsubmitted Not Done Inline Actions Noted. 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;
				}

				}
				epriestleyUnsubmitted Not Done Inline Actions This is largely philosophical, but two other approaches you could take here: Catch all of the exceptions, and throw `PhutilAggregateException` instead (probably best, but you lose the ability to catch specific exceptions). If you're throwing only one exception, consider throwing the first exception, not the last exception. I think the first exception tends to be the most relevant one more often (sometimes subsequent exceptions are really symptoms of the first exception, but the first exception is rarely/never a symptom of a later exception). But `AggregateException` is probably cleaner if no caller is trying to catch specific types of exceptions. epriestley: This is largely philosophical, but two other approaches you could take here: - Catch all of…
				20after4AuthorUnsubmitted Not Done Inline Actions I 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…