@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Cache Almanac URIs for repositories

Summary:
Ref T11954. This is kind of complex and I'm not sure I want to actually land it, but it gives us a fairly good improvement for clustered repositories so I'm leaning toward moving forward.

When we make (or receive) clustered repository requests, we must first load a bunch of stuff out of Almanac to figure out where to send the request (or if we can handle the request ourselves).

This involves several round trip queries into Almanac (service, device, interfaces, bindings, properties) and generally is fairly slow/expensive. The actual data we get out of it is just a list of URIs.

Caching this would be very easy, except that invalidating the cache is difficult, since editing any binding, property, interface, or device may invalidate the cache for indirectly connected services and repositories.

To address this, introduce `PhabricatorCacheEngine`, which is an extensible engine like `PhabricatorDestructionEngine` for propagating cache updates. It has two modes:

- Discover linked objects (that is: find related objects which may need to have caches invalidated).
- Invalidate caches (that is: nuke any caches which need to be nuked).

Both modes are extensible, so third-party code can build repository-dependent caches or whatever. This may be overkill but even if Almanac is the only thing we use it for it feels like a fairly clean solution to the problem.

With `CacheEngine`, make any edit to Almanac stuff propagate up to the Service, and then from the Service to any linked Repositories.

Once we hit repositories, invalidate their caches when Almanac changes.

Test Plan:
- Observed a 20-30ms performance improvement with `ab -n 100`.
- (The main page making Conduit calls also gets a performance improvement, although that's a little trickier to measure directly.)
- Added debugging code to the cache engine stuff to observe the linking and invalidation phases.
- Made invalidation throw; verified that editing properties, bindings, etc, properly invalidates the cache of any indirectly linked repositories.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11954

Differential Revision: https://secure.phabricator.com/D17000

+328 -24
+8
src/__phutil_library_map__.php
··· 22 22 'AlmanacBindingTransactionQuery' => 'applications/almanac/query/AlmanacBindingTransactionQuery.php', 23 23 'AlmanacBindingViewController' => 'applications/almanac/controller/AlmanacBindingViewController.php', 24 24 'AlmanacBindingsSearchEngineAttachment' => 'applications/almanac/engineextension/AlmanacBindingsSearchEngineAttachment.php', 25 + 'AlmanacCacheEngineExtension' => 'applications/almanac/engineextension/AlmanacCacheEngineExtension.php', 25 26 'AlmanacClusterDatabaseServiceType' => 'applications/almanac/servicetype/AlmanacClusterDatabaseServiceType.php', 26 27 'AlmanacClusterRepositoryServiceType' => 'applications/almanac/servicetype/AlmanacClusterRepositoryServiceType.php', 27 28 'AlmanacClusterServiceType' => 'applications/almanac/servicetype/AlmanacClusterServiceType.php', ··· 584 585 'DiffusionBrowseQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php', 585 586 'DiffusionBrowseResultSet' => 'applications/diffusion/data/DiffusionBrowseResultSet.php', 586 587 'DiffusionBrowseTableView' => 'applications/diffusion/view/DiffusionBrowseTableView.php', 588 + 'DiffusionCacheEngineExtension' => 'applications/diffusion/engineextension/DiffusionCacheEngineExtension.php', 587 589 'DiffusionCachedResolveRefsQuery' => 'applications/diffusion/query/DiffusionCachedResolveRefsQuery.php', 588 590 'DiffusionChangeController' => 'applications/diffusion/controller/DiffusionChangeController.php', 589 591 'DiffusionChangeHeraldFieldGroup' => 'applications/diffusion/herald/DiffusionChangeHeraldFieldGroup.php', ··· 2023 2025 'PhabricatorBulkContentSource' => 'infrastructure/daemon/contentsource/PhabricatorBulkContentSource.php', 2024 2026 'PhabricatorBusyUIExample' => 'applications/uiexample/examples/PhabricatorBusyUIExample.php', 2025 2027 'PhabricatorCacheDAO' => 'applications/cache/storage/PhabricatorCacheDAO.php', 2028 + 'PhabricatorCacheEngine' => 'applications/system/engine/PhabricatorCacheEngine.php', 2029 + 'PhabricatorCacheEngineExtension' => 'applications/system/engine/PhabricatorCacheEngineExtension.php', 2026 2030 'PhabricatorCacheGeneralGarbageCollector' => 'applications/cache/garbagecollector/PhabricatorCacheGeneralGarbageCollector.php', 2027 2031 'PhabricatorCacheManagementPurgeWorkflow' => 'applications/cache/management/PhabricatorCacheManagementPurgeWorkflow.php', 2028 2032 'PhabricatorCacheManagementWorkflow' => 'applications/cache/management/PhabricatorCacheManagementWorkflow.php', ··· 4571 4575 'AlmanacBindingTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 4572 4576 'AlmanacBindingViewController' => 'AlmanacServiceController', 4573 4577 'AlmanacBindingsSearchEngineAttachment' => 'AlmanacSearchEngineAttachment', 4578 + 'AlmanacCacheEngineExtension' => 'PhabricatorCacheEngineExtension', 4574 4579 'AlmanacClusterDatabaseServiceType' => 'AlmanacClusterServiceType', 4575 4580 'AlmanacClusterRepositoryServiceType' => 'AlmanacClusterServiceType', 4576 4581 'AlmanacClusterServiceType' => 'AlmanacServiceType', ··· 5225 5230 'DiffusionBrowseQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', 5226 5231 'DiffusionBrowseResultSet' => 'Phobject', 5227 5232 'DiffusionBrowseTableView' => 'DiffusionView', 5233 + 'DiffusionCacheEngineExtension' => 'PhabricatorCacheEngineExtension', 5228 5234 'DiffusionCachedResolveRefsQuery' => 'DiffusionLowLevelQuery', 5229 5235 'DiffusionChangeController' => 'DiffusionController', 5230 5236 'DiffusionChangeHeraldFieldGroup' => 'HeraldFieldGroup', ··· 6883 6889 'PhabricatorBulkContentSource' => 'PhabricatorContentSource', 6884 6890 'PhabricatorBusyUIExample' => 'PhabricatorUIExample', 6885 6891 'PhabricatorCacheDAO' => 'PhabricatorLiskDAO', 6892 + 'PhabricatorCacheEngine' => 'Phobject', 6893 + 'PhabricatorCacheEngineExtension' => 'Phobject', 6886 6894 'PhabricatorCacheGeneralGarbageCollector' => 'PhabricatorGarbageCollector', 6887 6895 'PhabricatorCacheManagementPurgeWorkflow' => 'PhabricatorCacheManagementWorkflow', 6888 6896 'PhabricatorCacheManagementWorkflow' => 'PhabricatorManagementWorkflow',
+53
src/applications/almanac/engineextension/AlmanacCacheEngineExtension.php
··· 1 + <?php 2 + 3 + final class AlmanacCacheEngineExtension 4 + extends PhabricatorCacheEngineExtension { 5 + 6 + const EXTENSIONKEY = 'almanac'; 7 + 8 + public function getExtensionName() { 9 + return pht('Almanac Core Objects'); 10 + } 11 + 12 + public function discoverLinkedObjects( 13 + PhabricatorCacheEngine $engine, 14 + array $objects) { 15 + $viewer = $engine->getViewer(); 16 + 17 + $results = array(); 18 + foreach ($this->selectObjects($objects, 'AlmanacBinding') as $object) { 19 + $results[] = $object->getServicePHID(); 20 + $results[] = $object->getDevicePHID(); 21 + $results[] = $object->getInterfacePHID(); 22 + } 23 + 24 + $devices = $this->selectObjects($objects, 'AlmanacDevice'); 25 + if ($devices) { 26 + $interfaces = id(new AlmanacInterfaceQuery()) 27 + ->setViewer($viewer) 28 + ->withDevicePHIDs(mpull($devices, 'getPHID')) 29 + ->execute(); 30 + foreach ($interfaces as $interface) { 31 + $results[] = $interface; 32 + } 33 + } 34 + 35 + foreach ($this->selectObjects($objects, 'AlmanacInterface') as $iface) { 36 + $results[] = $iface->getDevicePHID(); 37 + $results[] = $iface->getNetworkPHID(); 38 + } 39 + 40 + foreach ($this->selectObjects($objects, 'AlmanacProperty') as $object) { 41 + $results[] = $object->getObjectPHID(); 42 + } 43 + 44 + return $results; 45 + } 46 + 47 + public function deleteCaches( 48 + PhabricatorCacheEngine $engine, 49 + array $objects) { 50 + return; 51 + } 52 + 53 + }
+52
src/applications/diffusion/engineextension/DiffusionCacheEngineExtension.php
··· 1 + <?php 2 + 3 + final class DiffusionCacheEngineExtension 4 + extends PhabricatorCacheEngineExtension { 5 + 6 + const EXTENSIONKEY = 'diffusion'; 7 + 8 + public function getExtensionName() { 9 + return pht('Diffusion Repositories'); 10 + } 11 + 12 + public function discoverLinkedObjects( 13 + PhabricatorCacheEngine $engine, 14 + array $objects) { 15 + $viewer = $engine->getViewer(); 16 + $results = array(); 17 + 18 + // When an Almanac Service changes, update linked repositories. 19 + 20 + $services = $this->selectObjects($objects, 'AlmanacService'); 21 + if ($services) { 22 + $repositories = id(new PhabricatorRepositoryQuery()) 23 + ->setViewer($viewer) 24 + ->withAlmanacServicePHIDs(mpull($services, 'getPHID')) 25 + ->execute(); 26 + foreach ($repositories as $repository) { 27 + $results[] = $repository; 28 + } 29 + } 30 + 31 + return $results; 32 + } 33 + 34 + public function deleteCaches( 35 + PhabricatorCacheEngine $engine, 36 + array $objects) { 37 + 38 + $keys = array(); 39 + $repositories = $this->selectObjects($objects, 'PhabricatorRepository'); 40 + foreach ($repositories as $repository) { 41 + $keys[] = $repository->getAlmanacServiceCacheKey(); 42 + } 43 + 44 + $keys = array_filter($keys); 45 + 46 + if ($keys) { 47 + $cache = PhabricatorCaches::getMutableStructureCache(); 48 + $cache->deleteKeys($keys); 49 + } 50 + } 51 + 52 + }
+13
src/applications/repository/query/PhabricatorRepositoryQuery.php
··· 12 12 private $uris; 13 13 private $datasourceQuery; 14 14 private $slugs; 15 + private $almanacServicePHIDs; 15 16 16 17 private $numericIdentifiers; 17 18 private $callsignIdentifiers; ··· 131 132 132 133 public function withSlugs(array $slugs) { 133 134 $this->slugs = $slugs; 135 + return $this; 136 + } 137 + 138 + public function withAlmanacServicePHIDs(array $phids) { 139 + $this->almanacServicePHIDs = $phids; 134 140 return $this; 135 141 } 136 142 ··· 657 663 $conn, 658 664 'uri.repositoryURI IN (%Ls)', 659 665 $try_uris); 666 + } 667 + 668 + if ($this->almanacServicePHIDs !== null) { 669 + $where[] = qsprintf( 670 + $conn, 671 + 'r.almanacServicePHID IN (%Ls)', 672 + $this->almanacServicePHIDs); 660 673 } 661 674 662 675 return $where;
+62 -24
src/applications/repository/storage/PhabricatorRepository.php
··· 1865 1865 $never_proxy, 1866 1866 array $protocols) { 1867 1867 1868 - $service = $this->loadAlmanacService(); 1869 - if (!$service) { 1868 + $cache_key = $this->getAlmanacServiceCacheKey(); 1869 + if (!$cache_key) { 1870 1870 return null; 1871 1871 } 1872 1872 1873 - $bindings = $service->getActiveBindings(); 1874 - if (!$bindings) { 1875 - throw new Exception( 1876 - pht( 1877 - 'The Almanac service for this repository is not bound to any '. 1878 - 'interfaces.')); 1873 + $cache = PhabricatorCaches::getMutableStructureCache(); 1874 + $uris = $cache->getKey($cache_key, false); 1875 + 1876 + // If we haven't built the cache yet, build it now. 1877 + if ($uris === false) { 1878 + $uris = $this->buildAlmanacServiceURIs(); 1879 + $cache->setKey($cache_key, $uris); 1879 1880 } 1880 1881 1881 - $local_device = AlmanacKeys::getDeviceID(); 1882 + if ($uris === null) { 1883 + return null; 1884 + } 1882 1885 1886 + $local_device = AlmanacKeys::getDeviceID(); 1883 1887 if ($never_proxy && !$local_device) { 1884 1888 throw new Exception( 1885 1889 pht( ··· 1890 1894 1891 1895 $protocol_map = array_fuse($protocols); 1892 1896 1893 - $uris = array(); 1894 - foreach ($bindings as $binding) { 1895 - $iface = $binding->getInterface(); 1896 - 1897 + $results = array(); 1898 + foreach ($uris as $uri) { 1897 1899 // If we're never proxying this and it's locally satisfiable, return 1898 1900 // `null` to tell the caller to handle it locally. If we're allowed to 1899 1901 // proxy, we skip this check and may proxy the request to ourselves. ··· 1901 1903 // return `null`, and then the request will actually run.) 1902 1904 1903 1905 if ($local_device && $never_proxy) { 1904 - if ($iface->getDevice()->getName() == $local_device) { 1906 + if ($uri['device'] == $local_device) { 1905 1907 return null; 1906 1908 } 1907 1909 } 1908 1910 1909 - $uri = $this->getClusterRepositoryURIFromBinding($binding); 1910 - 1911 - $protocol = $uri->getProtocol(); 1912 - if (empty($protocol_map[$protocol])) { 1913 - continue; 1911 + if (isset($protocol_map[$uri['protocol']])) { 1912 + $results[] = new PhutilURI($uri['uri']); 1914 1913 } 1915 - 1916 - $uris[] = $uri; 1917 1914 } 1918 1915 1919 - if (!$uris) { 1916 + if (!$results) { 1920 1917 throw new Exception( 1921 1918 pht( 1922 1919 'The Almanac service for this repository is not bound to any '. ··· 1931 1928 'Cluster hosts must correctly route their intracluster requests.')); 1932 1929 } 1933 1930 1934 - shuffle($uris); 1935 - return head($uris); 1931 + shuffle($results); 1932 + return head($results); 1936 1933 } 1937 1934 1935 + public function getAlmanacServiceCacheKey() { 1936 + $service_phid = $this->getAlmanacServicePHID(); 1937 + if (!$service_phid) { 1938 + return null; 1939 + } 1940 + 1941 + $repository_phid = $this->getPHID(); 1942 + return "diffusion.repository({$repository_phid}).service({$service_phid})"; 1943 + } 1944 + 1945 + private function buildAlmanacServiceURIs() { 1946 + $service = $this->loadAlmanacService(); 1947 + if (!$service) { 1948 + return null; 1949 + } 1950 + 1951 + $bindings = $service->getActiveBindings(); 1952 + if (!$bindings) { 1953 + throw new Exception( 1954 + pht( 1955 + 'The Almanac service for this repository is not bound to any '. 1956 + 'interfaces.')); 1957 + } 1958 + 1959 + $uris = array(); 1960 + foreach ($bindings as $binding) { 1961 + $iface = $binding->getInterface(); 1962 + 1963 + $uri = $this->getClusterRepositoryURIFromBinding($binding); 1964 + $protocol = $uri->getProtocol(); 1965 + $device_name = $iface->getDevice()->getName(); 1966 + 1967 + $uris[] = array( 1968 + 'protocol' => $protocol, 1969 + 'uri' => (string)$uri, 1970 + 'device' => $device_name, 1971 + ); 1972 + } 1973 + 1974 + return $uris; 1975 + } 1938 1976 1939 1977 /** 1940 1978 * Build a new Conduit client in order to make a service call to this
+94
src/applications/system/engine/PhabricatorCacheEngine.php
··· 1 + <?php 2 + 3 + final class PhabricatorCacheEngine extends Phobject { 4 + 5 + public function getViewer() { 6 + return PhabricatorUser::getOmnipotentUser(); 7 + } 8 + 9 + public function updateObject($object) { 10 + $objects = array( 11 + $object->getPHID() => $object, 12 + ); 13 + 14 + $new_objects = $objects; 15 + while (true) { 16 + $discovered_objects = array(); 17 + $load = array(); 18 + 19 + $extensions = PhabricatorCacheEngineExtension::getAllExtensions(); 20 + foreach ($extensions as $key => $extension) { 21 + $discoveries = $extension->discoverLinkedObjects($this, $new_objects); 22 + if (!is_array($discoveries)) { 23 + throw new Exception( 24 + pht( 25 + 'Cache engine extension "%s" did not return a list of linked '. 26 + 'objects.', 27 + get_class($extension))); 28 + } 29 + 30 + foreach ($discoveries as $discovery) { 31 + if ($discovery === null) { 32 + // This is allowed because it makes writing extensions a lot 33 + // easier if they don't have to check that related PHIDs are 34 + // actually set to something. 35 + continue; 36 + } 37 + 38 + $is_phid = is_string($discovery); 39 + if ($is_phid) { 40 + $phid = $discovery; 41 + } else { 42 + $phid = $discovery->getPHID(); 43 + if (!$phid) { 44 + throw new Exception( 45 + pht( 46 + 'Cache engine extension "%s" returned object (of class '. 47 + '"%s") with no PHID.', 48 + get_class($extension), 49 + get_class($discovery))); 50 + } 51 + } 52 + 53 + if (isset($objects[$phid])) { 54 + continue; 55 + } 56 + 57 + if ($is_phid) { 58 + $load[$phid] = $phid; 59 + } else { 60 + $objects[$phid] = $discovery; 61 + $discovered_objects[$phid] = $discovery; 62 + 63 + // If another extension only knew about the PHID of this object, 64 + // we don't need to load it any more. 65 + unset($load[$phid]); 66 + } 67 + } 68 + } 69 + 70 + if ($load) { 71 + $load_objects = id(new PhabricatorObjectQuery()) 72 + ->setViewer($this->getViewer()) 73 + ->withPHIDs($load) 74 + ->execute(); 75 + foreach ($load_objects as $phid => $loaded_object) { 76 + $objects[$phid] = $loaded_object; 77 + $discovered_objects[$phid] = $loaded_object; 78 + } 79 + } 80 + 81 + // If we didn't find anything new to update, we're all set. 82 + if (!$discovered_objects) { 83 + break; 84 + } 85 + 86 + $new_objects = $discovered_objects; 87 + } 88 + 89 + foreach ($extensions as $extension) { 90 + $extension->deleteCaches($this, $objects); 91 + } 92 + } 93 + 94 + }
+42
src/applications/system/engine/PhabricatorCacheEngineExtension.php
··· 1 + <?php 2 + 3 + abstract class PhabricatorCacheEngineExtension extends Phobject { 4 + 5 + final public function getExtensionKey() { 6 + return $this->getPhobjectClassConstant('EXTENSIONKEY'); 7 + } 8 + 9 + abstract public function getExtensionName(); 10 + 11 + public function discoverLinkedObjects( 12 + PhabricatorCacheEngine $engine, 13 + array $objects) { 14 + return array(); 15 + } 16 + 17 + public function deleteCaches( 18 + PhabricatorCacheEngine $engine, 19 + array $objects) { 20 + return null; 21 + } 22 + 23 + final public static function getAllExtensions() { 24 + return id(new PhutilClassMapQuery()) 25 + ->setAncestorClass(__CLASS__) 26 + ->setUniqueMethod('getExtensionKey') 27 + ->execute(); 28 + } 29 + 30 + final public function selectObjects(array $objects, $class_name) { 31 + $results = array(); 32 + 33 + foreach ($objects as $phid => $object) { 34 + if ($object instanceof $class_name) { 35 + $results[$phid] = $object; 36 + } 37 + } 38 + 39 + return $results; 40 + } 41 + 42 + }
+4
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
··· 987 987 throw $ex; 988 988 } 989 989 990 + // If we need to perform cache engine updates, execute them now. 991 + id(new PhabricatorCacheEngine()) 992 + ->updateObject($object); 993 + 990 994 // Now that we've completely applied the core transaction set, try to apply 991 995 // Herald rules. Herald rules are allowed to either take direct actions on 992 996 // the database (like writing flags), or take indirect actions (like saving