Merge pull request #17 from aternosorg/relative-refresh-threshold

Make refreshThreshold relative to refreshTime

Author: JulianVennen

src/AbstractLock.php

@@ -62,8 +62,8 @@ public static function setDefaultIdentifier(?string $defaultIdentifier = null):
      * @param int $waitTime Time in seconds to wait for existing locks to be released.
      * @param int|null $refreshTime Duration in seconds the timeout should be set to when refreshing the lock.
      * If null the initial timeout will be used.
-     * @param int $refreshThreshold Maximum duration in seconds the existing lock may be valid for to be refreshed.
-     * If the lock is valid for longer than this time, the lock will not be refreshed.
+     * @param float $refreshThreshold Maximum percentage of the refreshTime the existing lock may still be valid for
+     * to be refreshed. If the lock is valid for longer than this time, it won't be refreshed.
      * @param bool $breakOnDestruct Automatically try to break the lock on destruct if possible
      */
     public function __construct(
@@ -73,7 +73,7 @@ public function __construct(
         protected int $time = 120,
         protected int $waitTime = 300,
         protected ?int $refreshTime = null,
-        protected int $refreshThreshold = 30,
+        protected float $refreshThreshold = 0.5,
         protected bool $breakOnDestruct = true,
     )
     {
@@ -185,6 +185,16 @@ public function getRefreshTime(): ?int
         return $this->refreshTime;
     }
 
+    /**
+     * Duration in seconds the timeout should be set to when refreshing the lock.
+     * Same as {@link AbstractLock::getRefreshTime()} except that the lock time is used if the refresh time was not set.
+     * @return int
+     */
+    public function getEffectiveRefreshTime(): int
+    {
+        return $this->refreshTime ?? $this->time;
+    }
+
     /**
      * Duration in seconds the timeout should be set to when refreshing the lock.
      * If null the initial timeout will be used.
@@ -198,22 +208,22 @@ public function setRefreshTime(?int $refreshTime): static
     }
 
     /**
-     * Maximum duration in seconds the existing lock may be valid for to be refreshed. If the lock is valid for longer
-     * than this time, the lock will not be refreshed.
-     * @return int
+     * Maximum percentage of the refreshTime the existing lock may still be valid for to be refreshed. If the lock is
+     * valid for longer than this time, it won't be refreshed.
+     * @return float
      */
-    public function getRefreshThreshold(): int
+    public function getRefreshThreshold(): float
     {
         return $this->refreshThreshold;
     }
 
     /**
-     * Maximum duration in seconds the existing lock may be valid for to be refreshed. If the lock is valid for longer
-     * than this time, the lock will not be refreshed.
-     * @param int $refreshThreshold
+     * Maximum percentage of the refreshTime the existing lock may still be valid for to be refreshed. If the lock is
+     * valid for longer than this time, it won't be refreshed.
+     * @param float $refreshThreshold
      * @return $this
      */
-    public function setRefreshThreshold(int $refreshThreshold): static
+    public function setRefreshThreshold(float $refreshThreshold): static
     {
         $this->refreshThreshold = $refreshThreshold;
         return $this;

src/Lock.php

@@ -190,8 +190,8 @@ public static function setDelayPerUnavailableRetry(int $delayPerRetry): void
      * @param int $waitTime Time in seconds to wait for existing locks to be released.
      * @param int|null $refreshTime Duration in seconds the timeout should be set to when refreshing the lock.
      * If null the initial timeout will be used.
-     * @param int $refreshThreshold Maximum duration in seconds the existing lock may be valid for to be refreshed.
-     * If the lock is valid for longer than this time, the lock will not be refreshed.
+     * @param float $refreshThreshold Maximum percentage of the refreshTime the existing lock may still be valid for
+     * to be refreshed. If the lock is valid for longer than this time, it won't be refreshed.
      * @param bool $breakOnDestruct Automatically try to break the lock on destruct if possible
      */
     public function __construct(
@@ -201,7 +201,7 @@ public function __construct(
         int $time = 120,
         int $waitTime = 300,
         ?int $refreshTime = null,
-        int $refreshThreshold = 30,
+        float $refreshThreshold = 0.5,
         bool $breakOnDestruct = true,
     )
     {
@@ -280,7 +280,7 @@ public function getRemainingLockDuration(): int
      */
     public function refresh(): bool
     {
-        if ($this->refreshThreshold > 0 && $this->getRemainingLockDuration() > $this->refreshThreshold) {
+        if (!$this->shouldRefresh()) {
             return true;
         }
 
@@ -292,7 +292,7 @@ public function refresh(): bool
                 return false;
             }
 
-            $retry = !$this->addOrUpdateLock($this->refreshTime ?? $this->time);
+            $retry = !$this->addOrUpdateLock($this->getEffectiveRefreshTime());
         } while ($retry);
         return true;
     }
@@ -327,6 +327,21 @@ protected function generateLock(): LockEntry
         return new LockEntry($this->identifier, time() + $this->time, $this->exclusive);
     }
 
+    /**
+     * Returns true, if the remaining lock duration is less than the refresh threshold.
+     * @return bool
+     */
+    protected function shouldRefresh(): bool
+    {
+        if ($this->refreshThreshold <= 0) {
+            return true;
+        }
+
+        $remaining = $this->getRemainingLockDuration();
+        $threshold = $this->refreshThreshold * $this->getEffectiveRefreshTime();
+        return $remaining < $threshold;
+    }
+
     /**
      * Remove a lock from the locking array and save the locks
      *

test/LockTest.php

@@ -113,30 +113,45 @@ public function testGetRefreshTime(): void
         $this->assertEquals(5, $lock->getRefreshTime());
     }
 
+    public function testGetEffectiveRefreshTime(): void
+    {
+        $lock = new Lock("key");
+        $this->assertEquals(120, $lock->getEffectiveRefreshTime());
+
+        $lock = new Lock("key", time: 10);
+        $this->assertEquals(10, $lock->getEffectiveRefreshTime());
+
+        $lock = new Lock("key", "identifier", false, 10, 0, 5);
+        $this->assertEquals(5, $lock->getEffectiveRefreshTime());
+    }
+
     public function testSetRefreshTime(): void
     {
         $lock = new Lock("key");
         $this->assertEquals(null, $lock->getRefreshTime());
+        $this->assertEquals(120, $lock->getEffectiveRefreshTime());
         $lock->setRefreshTime(5);
         $this->assertEquals(5, $lock->getRefreshTime());
+        $this->assertEquals(5, $lock->getEffectiveRefreshTime());
         $lock->setRefreshTime(null);
         $this->assertEquals(null, $lock->getRefreshTime());
+        $this->assertEquals(120, $lock->getEffectiveRefreshTime());
     }
 
     public function testGetRefreshThreshold(): void
     {
         $lock = new Lock("key");
-        $this->assertEquals(30, $lock->getRefreshThreshold());
-        $lock = new Lock("key", "identifier", false, 10, 0, 5, 2);
-        $this->assertEquals(2, $lock->getRefreshThreshold());
+        $this->assertEquals(0.5, $lock->getRefreshThreshold());
+        $lock = new Lock("key", "identifier", false, 10, 0, 5, 0.25);
+        $this->assertEquals(0.25, $lock->getRefreshThreshold());
     }
 
     public function testSetRefreshThreshold(): void
     {
         $lock = new Lock("key");
-        $this->assertEquals(30, $lock->getRefreshThreshold());
-        $lock->setRefreshThreshold(5);
-        $this->assertEquals(5, $lock->getRefreshThreshold());
+        $this->assertEquals(0.5, $lock->getRefreshThreshold());
+        $lock->setRefreshThreshold(0.25);
+        $this->assertEquals(0.25, $lock->getRefreshThreshold());
     }
 
     public function testShouldBreakOnDestruct(): void
@@ -248,7 +263,7 @@ public function testRefreshLockThreshold(): void
         $key = $this->getRandomString();
         $identifier = $this->getRandomString();
 
-        $lock = new Lock($key, $identifier, false, 10, 0, refreshThreshold: 5);
+        $lock = new Lock($key, $identifier, false, 10, 0, refreshThreshold: 0.5);
         $lock->lock();
         $this->assertTrue($lock->isLocked());
         sleep(3);