revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 01:08:50 +02:00
Code Issues Releases Wiki Activity

Try to route cluster writes to nodes which won't need to synchronize first

Browse source 
Summary:
Ref T13109. Ref T13202. See PHI905. See PHI889. When we receive a write to a repository cluster, we currently send it to a random writable node.

Instead, we can prefer:

  - the node currently holding the write lock; or
  - any node which is already up to date.

These should simply be better nodes to take writes in all cases. The write lock is global for the repository, so there's no scaling benefit to spreading writes across different nodes, and these particular nodes will be able to accept the write more quickly.

Test Plan:
  - This is observable by using `fprintf(STDERR, "%s\n", ...)` in the logic, then running `git push`. I'd like to pull this routing logic out of `PhabricatorRepository` at some point, probably into a dedicated `ClusterURIQuery` sort of class, but that is a larger change.
  - Added some `fprintf(...)` stuff around which nodes were being selected.
  - Added a `sleep(10)` after grabbing the write lock.
  - In one window, pushed. Then pushed in a second window.
    - Saw the second window select the lock holder as the write target based on it currently holding the lock.
    - Without a concurrent push, saw pushes select up-to-date nodes based on their up-to-date-ness.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: joshuaspence, timhirsh

Maniphest Tasks: T13202, T13109

Differential Revision: https://secure.phabricator.com/D19734
This commit is contained in:
epriestley 2018-10-05 13:17:17 -07:00
parent 0a51bc4f05
commit 51073b972e
1 changed files with 64 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

66
src/applications/repository/storage/PhabricatorRepository.php
View file

@ -2010,12 +2010,72 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
}
}
shuffle($results);
if ($writable) {
$results = $this->sortWritableAlmanacServiceURIs($results);
} else {
shuffle($results);
}
$result = head($results);
return $result['uri'];
}
private function sortWritableAlmanacServiceURIs(array $results) {
// See T13109 for discussion of how this method routes requests.
// In the absence of other rules, we'll send traffic to devices randomly.
// We also want to select randomly among nodes which are equally good
// candidates to receive the write, and accomplish that by shuffling the
// list up front.
shuffle($results);
$order = array();
// If some device is currently holding the write lock, send all requests
// to that device. We're trying to queue writes on a single device so they
// do not need to wait for read synchronization after earlier writes
// complete.
$writer = PhabricatorRepositoryWorkingCopyVersion::loadWriter(
$this->getPHID());
if ($writer) {
$device_phid = $writer->getWriteProperty('devicePHID');
foreach ($results as $key => $result) {
if ($result['devicePHID'] === $device_phid) {
$order[] = $key;
}
}
}
// If no device is currently holding the write lock, try to send requests
// to a device which is already up to date and will not need to synchronize
// before it can accept the write.
$versions = PhabricatorRepositoryWorkingCopyVersion::loadVersions(
$this->getPHID());
if ($versions) {
$max_version = (int)max(mpull($versions, 'getRepositoryVersion'));
$max_devices = array();
foreach ($versions as $version) {
if ($version->getRepositoryVersion() == $max_version) {
$max_devices[] = $version->getDevicePHID();
}
}
$max_devices = array_fuse($max_devices);
foreach ($results as $key => $result) {
if (isset($max_devices[$result['devicePHID']])) {
$order[] = $key;
}
}
}
// Reorder the results, putting any we've selected as preferred targets for
// the write at the head of the list.
$results = array_select_keys($results, $order) + $results;
return $results;
}
public function supportsSynchronization() {
// TODO: For now, this is only supported for Git.
if (!$this->isGit()) {
@ -2036,7 +2096,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
$parts = array(
"repo({$repository_phid})",
"serv({$service_phid})",
'v2',
'v3',
);
return implode('.', $parts);
@ -2063,12 +2123,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
$uri = $this->getClusterRepositoryURIFromBinding($binding);
$protocol = $uri->getProtocol();
$device_name = $iface->getDevice()->getName();
$device_phid = $iface->getDevice()->getPHID();
$uris[] = array(
'protocol' => $protocol,
'uri' => (string)$uri,
'device' => $device_name,
'writable' => (bool)$binding->getAlmanacPropertyValue('writable'),
'devicePHID' => $device_phid,
);
}