From 211715a77931f0f0e59da311cad673341cbf2b3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Tue, 10 Dec 2024 14:06:21 +0100 Subject: [PATCH 1/2] Stop acquiring distributed lock as soon as majority cannot be met --- src/Mutex/DistributedMutex.php | 9 +++++++++ tests/Mutex/DistributedMutexTest.php | 10 +++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/Mutex/DistributedMutex.php b/src/Mutex/DistributedMutex.php index 991e81f..c1fd126 100644 --- a/src/Mutex/DistributedMutex.php +++ b/src/Mutex/DistributedMutex.php @@ -52,6 +52,7 @@ protected function acquireWithToken(string $key, float $expireTimeout) // 2. $acquiredIndexes = []; + $notAcquired = 0; $errored = 0; $exception = null; foreach ($this->mutexes as $index => $mutex) { @@ -68,6 +69,14 @@ protected function acquireWithToken(string $key, float $expireTimeout) ++$errored; } + + if (end($acquiredIndexes) !== $index) { + ++$notAcquired; + } + + if (!$this->isCountMajority(count($this->mutexes) - $notAcquired)) { + break; + } } // 3. diff --git a/tests/Mutex/DistributedMutexTest.php b/tests/Mutex/DistributedMutexTest.php index be96d90..4fefab5 100644 --- a/tests/Mutex/DistributedMutexTest.php +++ b/tests/Mutex/DistributedMutexTest.php @@ -93,7 +93,7 @@ public function testTooFewServerToAcquire(int $count, int $available): void $mutex = $this->createDistributedMutexMock($count); $i = 0; - $mutex->expects(self::exactly($count)) + $mutex->expects(self::atMost((int) floor($count / 2) + $count - $available)) ->method('acquireMutex') ->willReturnCallback(static function () use (&$i, $available) { if ($i++ < $available) { @@ -307,6 +307,9 @@ public static function provideMinorityCases(): iterable yield [4, 0]; yield [4, 1]; yield [4, 2]; + yield [5, 2]; + yield [6, 2]; + yield [6, 3]; } /** @@ -321,6 +324,7 @@ public static function provideMajorityCases(): iterable yield [3, 2]; yield [3, 3]; yield [4, 3]; + yield [5, 3]; } public function testAcquireMutexLogger(): void @@ -329,12 +333,12 @@ public function testAcquireMutexLogger(): void $logger = $this->createMock(LoggerInterface::class); $mutex->setLogger($logger); - $mutex->expects(self::exactly(3)) + $mutex->expects(self::exactly(2)) ->method('acquireMutex') ->with(self::isInstanceOf(AbstractSpinlockMutex::class), 'distributed', 1.0, \INF) ->willThrowException($this->createMock(/* PredisException::class */ LockAcquireException::class)); - $logger->expects(self::exactly(3)) + $logger->expects(self::exactly(2)) ->method('warning') ->with('Could not set {key} = {token} at server #{index}', self::anything()); From 7b755748014ee98a83a730720e102591451ccd5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Tue, 10 Dec 2024 14:50:07 +0100 Subject: [PATCH 2/2] acquire mutexes in random order --- src/Mutex/DistributedMutex.php | 18 +++++++++++++++++- tests/Mutex/DistributedMutexTest.php | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/Mutex/DistributedMutex.php b/src/Mutex/DistributedMutex.php index c1fd126..8ccc862 100644 --- a/src/Mutex/DistributedMutex.php +++ b/src/Mutex/DistributedMutex.php @@ -55,7 +55,7 @@ protected function acquireWithToken(string $key, float $expireTimeout) $notAcquired = 0; $errored = 0; $exception = null; - foreach ($this->mutexes as $index => $mutex) { + foreach ($this->getMutexesInRandomOrder() as $index => $mutex) { try { if ($this->acquireMutex($mutex, $key, $acquireTimeout - (microtime(true) - $startTs), $expireTimeout)) { $acquiredIndexes[] = $index; @@ -138,6 +138,22 @@ protected function releaseWithToken(string $key, string $token): bool } } + /** + * @return array + */ + private function getMutexesInRandomOrder(): array + { + $indexes = array_keys($this->mutexes); + shuffle($indexes); + + $res = []; + foreach ($indexes as $index) { + $res[$index] = $this->mutexes[$index]; + } + + return $res; + } + /** * @return bool True if the count is the majority */ diff --git a/tests/Mutex/DistributedMutexTest.php b/tests/Mutex/DistributedMutexTest.php index 4fefab5..883c85c 100644 --- a/tests/Mutex/DistributedMutexTest.php +++ b/tests/Mutex/DistributedMutexTest.php @@ -93,7 +93,7 @@ public function testTooFewServerToAcquire(int $count, int $available): void $mutex = $this->createDistributedMutexMock($count); $i = 0; - $mutex->expects(self::atMost((int) floor($count / 2) + $count - $available)) + $mutex->expects(self::atMost((int) floor($count / 2) + $count - $available)) ->method('acquireMutex') ->willReturnCallback(static function () use (&$i, $available) { if ($i++ < $available) {