Skip to content

Commit 788962f

Browse files
antonkomarevantonkomarev
committed
Rewrite logic to two keyed lock
1 parent 8798e5e commit 788962f

File tree

5 files changed

+79
-40
lines changed

5 files changed

+79
-40
lines changed

‎src/LockId/PostgresLockId.php

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,31 @@ final class PostgresLockId
2121
private const DB_INT32_VALUE_MAX = 2_147_483_647;
2222

2323
public function __construct(
24-
public readonly int $id,
24+
public readonly int $classId,
25+
public readonly int $objectId,
2526
public readonly string $humanReadableValue = '',
2627
) {
27-
if ($id < self::DB_INT32_VALUE_MIN) {
28-
throw new InvalidArgumentException('Out of bound exception (id is too small)');
28+
if ($classId < self::DB_INT32_VALUE_MIN) {
29+
throw new InvalidArgumentException('Out of bound exception (classId is too small)');
2930
}
30-
if ($id > self::DB_INT32_VALUE_MAX) {
31-
throw new InvalidArgumentException('Out of bound exception (id is too big)');
31+
if ($classId > self::DB_INT32_VALUE_MAX) {
32+
throw new InvalidArgumentException('Out of bound exception (classId is too big)');
33+
}
34+
if ($objectId < self::DB_INT32_VALUE_MIN) {
35+
throw new InvalidArgumentException('Out of bound exception (objectId is too small)');
36+
}
37+
if ($objectId > self::DB_INT32_VALUE_MAX) {
38+
throw new InvalidArgumentException('Out of bound exception (objectId is too big)');
3239
}
3340
}
3441

3542
public static function fromLockId(
3643
LockId $lockId,
3744
): self {
38-
$lockStringId = (string)$lockId;
39-
4045
return new self(
41-
id: self::convertStringToSignedInt32($lockStringId),
42-
humanReadableValue: $lockStringId,
46+
classId: self::convertStringToSignedInt32($lockId->key),
47+
objectId: self::convertStringToSignedInt32($lockId->value),
48+
humanReadableValue: (string)$lockId,
4349
);
4450
}
4551

‎src/Locker/PostgresAdvisoryLocker.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,13 @@ public function tryAcquireLock(
2626
// TODO: Need to sanitize humanReadableValue?
2727
$statement = $dbConnection->prepare(
2828
<<<SQL
29-
SELECT PG_TRY_ADVISORY_LOCK(:lock_id); -- $postgresLockId->humanReadableValue
29+
SELECT PG_TRY_ADVISORY_LOCK(:class_id, :object_id); -- $postgresLockId->humanReadableValue
3030
SQL,
3131
);
3232
$statement->execute(
3333
[
34-
'lock_id' => $postgresLockId->id,
34+
'class_id' => $postgresLockId->classId,
35+
'object_id' => $postgresLockId->objectId,
3536
],
3637
);
3738

@@ -53,12 +54,13 @@ public function tryAcquireLockWithinTransaction(
5354
// TODO: Need to sanitize humanReadableValue?
5455
$statement = $dbConnection->prepare(
5556
<<<SQL
56-
SELECT PG_TRY_ADVISORY_XACT_LOCK(:lock_id); -- $postgresLockId->humanReadableValue
57+
SELECT PG_TRY_ADVISORY_XACT_LOCK(:class_id, :object_id); -- $postgresLockId->humanReadableValue
5758
SQL,
5859
);
5960
$statement->execute(
6061
[
61-
'lock_id' => $postgresLockId->id,
62+
'class_id' => $postgresLockId->classId,
63+
'object_id' => $postgresLockId->objectId,
6264
],
6365
);
6466

@@ -71,12 +73,13 @@ public function releaseLock(
7173
): bool {
7274
$statement = $dbConnection->prepare(
7375
<<<SQL
74-
SELECT PG_ADVISORY_UNLOCK(:lock_id); -- $postgresLockId->humanReadableValue
76+
SELECT PG_ADVISORY_UNLOCK(:class_id, :object_id); -- $postgresLockId->humanReadableValue
7577
SQL,
7678
);
7779
$statement->execute(
7880
[
79-
'lock_id' => $postgresLockId->id,
81+
'class_id' => $postgresLockId->classId,
82+
'object_id' => $postgresLockId->objectId,
8083
],
8184
);
8285

‎test/Integration/AbstractIntegrationTestCase.php

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,6 @@ private function findPostgresAdvisoryLockInConnection(
9191
): object | null {
9292
// For one-argument advisory locks, Postgres stores the signed 64-bit key as two 32-bit integers:
9393
// classid = high 32 bits, objid = low 32 bits.
94-
$lockClassId = ($postgresLockId->id >> 32) & 0xFFFFFFFF;
95-
$lockObjectId = $postgresLockId->id & 0xFFFFFFFF;
96-
97-
// Convert to signed 32-bit if necessary (Postgres stores as signed)
98-
if ($lockClassId > 0x7FFFFFFF) {
99-
$lockClassId -= 0x100000000;
100-
}
101-
if ($lockObjectId > 0x7FFFFFFF) {
102-
$lockObjectId -= 0x100000000;
103-
}
10494

10595
$statement = $dbConnection->prepare(
10696
<<<'SQL'
@@ -116,9 +106,9 @@ private function findPostgresAdvisoryLockInConnection(
116106
);
117107
$statement->execute(
118108
[
119-
'lock_class_id' => $lockClassId,
120-
'lock_object_id' => $lockObjectId,
121-
'lock_object_subid' => 1, // For one keyed value
109+
'lock_class_id' => $postgresLockId->classId,
110+
'lock_object_id' => $postgresLockId->objectId,
111+
'lock_object_subid' => 2, // Using two keyed locks
122112
'connection_pid' => $dbConnection->pgsqlGetPid(),
123113
'mode' => self::MODE_EXCLUSIVE,
124114
],

‎test/Integration/Locker/PostgresAdvisoryLockerTest.php

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ public function test_it_can_acquire_lock(): void
3737
$this->assertPgAdvisoryLocksCount(1);
3838
}
3939

40-
public function test_it_can_acquire_lock_with_smallest_lock_id(): void
40+
public function test_it_can_acquire_lock_with_min_class_id(): void
4141
{
4242
$locker = $this->initLocker();
4343
$dbConnection = $this->initPostgresPdoConnection();
44-
$postgresLockId = new PostgresLockId(self::DB_INT32_VALUE_MIN);
44+
$postgresLockId = new PostgresLockId(self::DB_INT32_VALUE_MIN, 0);
4545

4646
$isLockAcquired = $locker->tryAcquireLock($dbConnection, $postgresLockId);
4747

@@ -50,11 +50,37 @@ public function test_it_can_acquire_lock_with_smallest_lock_id(): void
5050
$this->assertPgAdvisoryLocksCount(1);
5151
}
5252

53-
public function test_it_can_acquire_lock_with_biggest_lock_id(): void
53+
public function test_it_can_acquire_lock_with_max_class_id(): void
5454
{
5555
$locker = $this->initLocker();
5656
$dbConnection = $this->initPostgresPdoConnection();
57-
$postgresLockId = new PostgresLockId(self::DB_INT32_VALUE_MAX);
57+
$postgresLockId = new PostgresLockId(self::DB_INT32_VALUE_MAX, 0);
58+
59+
$isLockAcquired = $locker->tryAcquireLock($dbConnection, $postgresLockId);
60+
61+
$this->assertTrue($isLockAcquired);
62+
$this->assertPgAdvisoryLockExistsInConnection($dbConnection, $postgresLockId);
63+
$this->assertPgAdvisoryLocksCount(1);
64+
}
65+
66+
public function test_it_can_acquire_lock_with_min_object_id(): void
67+
{
68+
$locker = $this->initLocker();
69+
$dbConnection = $this->initPostgresPdoConnection();
70+
$postgresLockId = new PostgresLockId(0, self::DB_INT32_VALUE_MIN);
71+
72+
$isLockAcquired = $locker->tryAcquireLock($dbConnection, $postgresLockId);
73+
74+
$this->assertTrue($isLockAcquired);
75+
$this->assertPgAdvisoryLockExistsInConnection($dbConnection, $postgresLockId);
76+
$this->assertPgAdvisoryLocksCount(1);
77+
}
78+
79+
public function test_it_can_acquire_lock_with_max_object_id(): void
80+
{
81+
$locker = $this->initLocker();
82+
$dbConnection = $this->initPostgresPdoConnection();
83+
$postgresLockId = new PostgresLockId(0, self::DB_INT32_VALUE_MAX);
5884

5985
$isLockAcquired = $locker->tryAcquireLock($dbConnection, $postgresLockId);
6086

‎test/Unit/LockId/PostgresLockIdTest.php

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,32 @@ final class PostgresLockIdTest extends AbstractUnitTestCase
2222
private const DB_INT32_VALUE_MIN = -2_147_483_648;
2323
private const DB_INT32_VALUE_MAX = 2_147_483_647;
2424

25-
public function test_it_can_create_postgres_lock_id_with_min_id(): void
25+
public function test_it_can_create_postgres_lock_id_with_min_class_id(): void
2626
{
27-
$lockId = new PostgresLockId(self::DB_INT32_VALUE_MIN);
27+
$lockId = new PostgresLockId(self::DB_INT32_VALUE_MIN, 0);
2828

29-
$this->assertSame(self::DB_INT32_VALUE_MIN, $lockId->id);
29+
$this->assertSame(self::DB_INT32_VALUE_MIN, $lockId->classId);
3030
}
3131

32-
public function test_it_can_create_postgres_lock_id_with_max_id(): void
32+
public function test_it_can_create_postgres_lock_id_with_max_class_id(): void
3333
{
34-
$lockId = new PostgresLockId(self::DB_INT32_VALUE_MAX);
34+
$lockId = new PostgresLockId(self::DB_INT32_VALUE_MAX, 0);
3535

36-
$this->assertSame(self::DB_INT32_VALUE_MAX, $lockId->id);
36+
$this->assertSame(self::DB_INT32_VALUE_MAX, $lockId->classId);
37+
}
38+
39+
public function test_it_can_create_postgres_lock_id_with_min_object_id(): void
40+
{
41+
$lockId = new PostgresLockId(0, self::DB_INT32_VALUE_MIN);
42+
43+
$this->assertSame(self::DB_INT32_VALUE_MIN, $lockId->objectId);
44+
}
45+
46+
public function test_it_can_create_postgres_lock_id_with_max_object_id(): void
47+
{
48+
$lockId = new PostgresLockId(0, self::DB_INT32_VALUE_MAX);
49+
50+
$this->assertSame(self::DB_INT32_VALUE_MAX, $lockId->objectId);
3751
}
3852

3953
public function test_it_can_create_postgres_lock_id_from_lock_id(): void
@@ -42,7 +56,7 @@ public function test_it_can_create_postgres_lock_id_from_lock_id(): void
4256

4357
$postgresLockId = PostgresLockId::fromLockId($lockId);
4458

45-
$this->assertSame(-662733300, $postgresLockId->id);
59+
$this->assertSame(-662733300, $postgresLockId->classId);
4660
}
4761

4862
public function test_it_can_create_postgres_lock_id_from_lock_id_with_value(): void
@@ -51,6 +65,6 @@ public function test_it_can_create_postgres_lock_id_from_lock_id_with_value(): v
5165

5266
$postgresLockId = PostgresLockId::fromLockId($lockId);
5367

54-
$this->assertSame(782632948, $postgresLockId->id);
68+
$this->assertSame(-662733300, $postgresLockId->classId);
5569
}
5670
}

0 commit comments

Comments
 (0)