Skip to content

Commit fd367ad

Browse files
author
epriestley
committed
Parameterize PhabricatorGlobalLock
Summary: Ref T13096. Currently, we do a fair amount of clever digesting and string manipulation to build lock names which are less than 64 characters long while still being reasonably readable. Instead, do more of this automatically. This will let lock acquisition become simpler and make it more possible to build a useful lock log. Test Plan: Ran `bin/repository update`, saw a reasonable lock acquire and release. Maniphest Tasks: T13096 Differential Revision: https://secure.phabricator.com/D19173
1 parent f31975f commit fd367ad

File tree

2 files changed

+33
-18
lines changed

2 files changed

+33
-18
lines changed

src/applications/repository/engine/PhabricatorRepositoryEngine.php

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,18 @@ protected function newRepositoryLock(
5656
$lock_key,
5757
$lock_device_only) {
5858

59-
$lock_parts = array();
60-
$lock_parts[] = $lock_key;
61-
$lock_parts[] = $repository->getID();
59+
$lock_parts = array(
60+
'repositoryPHID' => $repository->getPHID(),
61+
);
6262

6363
if ($lock_device_only) {
6464
$device = AlmanacKeys::getLiveDevice();
6565
if ($device) {
66-
$lock_parts[] = $device->getID();
66+
$lock_parts['devicePHID'] = $device->getPHID();
6767
}
6868
}
6969

70-
$lock_name = implode(':', $lock_parts);
71-
return PhabricatorGlobalLock::newLock($lock_name);
70+
return PhabricatorGlobalLock::newLock($lock_key, $lock_parts);
7271
}
7372

7473

src/infrastructure/util/PhabricatorGlobalLock.php

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
*/
2929
final class PhabricatorGlobalLock extends PhutilLock {
3030

31+
private $parameters;
3132
private $conn;
3233
private $isExternalConnection = false;
3334

@@ -37,27 +38,42 @@ final class PhabricatorGlobalLock extends PhutilLock {
3738
/* -( Constructing Locks )------------------------------------------------- */
3839

3940

40-
public static function newLock($name) {
41+
public static function newLock($name, $parameters = array()) {
4142
$namespace = PhabricatorLiskDAO::getStorageNamespace();
4243
$namespace = PhabricatorHash::digestToLength($namespace, 20);
4344

44-
$full_name = 'ph:'.$namespace.':'.$name;
45-
46-
$length_limit = 64;
47-
if (strlen($full_name) > $length_limit) {
48-
throw new Exception(
49-
pht(
50-
'Lock name "%s" is too long (full lock name is "%s"). The '.
51-
'full lock name must not be longer than %s bytes.',
52-
$name,
53-
$full_name,
54-
new PhutilNumber($length_limit)));
45+
$parts = array();
46+
ksort($parameters);
47+
foreach ($parameters as $key => $parameter) {
48+
if (!preg_match('/^[a-zA-Z0-9]+\z/', $key)) {
49+
throw new Exception(
50+
pht(
51+
'Lock parameter key "%s" must be alphanumeric.',
52+
$key));
53+
}
54+
55+
if (!is_scalar($parameter) && !is_null($parameter)) {
56+
throw new Exception(
57+
pht(
58+
'Lock parameter for key "%s" must be a scalar.',
59+
$key));
60+
}
61+
62+
$value = phutil_json_encode($parameter);
63+
$parts[] = "{$key}={$value}";
5564
}
65+
$parts = implode(', ', $parts);
5666

67+
$local = "{$name}({$parts})";
68+
$local = PhabricatorHash::digestToLength($local, 20);
69+
70+
$full_name = "ph:{$namespace}:{$local}";
5771
$lock = self::getLock($full_name);
5872
if (!$lock) {
5973
$lock = new PhabricatorGlobalLock($full_name);
6074
self::registerLock($lock);
75+
76+
$lock->parameters = $parameters;
6177
}
6278

6379
return $lock;

0 commit comments

Comments
 (0)