Improve distributed locking (#77)

Author: mvorisek

src/Mutex/DistributedMutex.php

@@ -52,9 +52,10 @@ protected function acquireWithToken(string $key, float $expireTimeout)
 
         // 2.
         $acquiredIndexes = [];
+        $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;
@@ -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.
@@ -129,6 +138,22 @@ protected function releaseWithToken(string $key, string $token): bool
         }
     }
 
+    /**
+     * @return array<int, AbstractSpinlockMutex>
+     */
+    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
      */

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());