Merge pull request #75 from zynga/AddRetryOnLocks

Add retry on locks

src/Zynga/Framework/Cache/V2/Driver/Memcache.hh

@@ -21,6 +21,10 @@ class Memcache extends DriverBase implements MemcacheDriverInterface {
   private DriverConfigInterface $_config;
   // Map used to keep track of hosts that have been registered to avoid duplicates
   private Map<string, int> $_registeredHosts;
+
+  // Operations like set and delete will retry their operation
+  const int OPERATION_ATTEMPTS_MAX = 100; // 100 * 10000 = 1s max wait time.
+  const int OPERATION_TIMEOUT_AMOUNT_MICRO_SECONDS = 10000;
 
   public function __construct(DriverConfigInterface $config) {
     $this->_config = $config;
@@ -94,9 +98,16 @@ class Memcache extends DriverBase implements MemcacheDriverInterface {
 
       $this->connect();
 
-      $success = $this->_memcache->set($key, $value, $flags, $ttl);
-
-      return $success;
+      for ($retryCount = 0; $retryCount < self::OPERATION_ATTEMPTS_MAX; $retryCount++) {
+        $success = $this->_memcache->set($key, $value, $flags, $ttl);
+        if ($success == true) {
+          return true;
+        }
+
+        usleep(self::OPERATION_TIMEOUT_AMOUNT_MICRO_SECONDS);
+      }
+
+      return false;
 
     } catch (Exception $e) {
       throw $e;
@@ -121,12 +132,16 @@ class Memcache extends DriverBase implements MemcacheDriverInterface {
     try {
 
       $this->connect();
-
-      $value = $this->_memcache->delete($key);
-
-      if ($value == true) {
-        return true;
+
+      for ($retryCount = 0; $retryCount < self::OPERATION_ATTEMPTS_MAX; $retryCount++) {
+        $success = $this->_memcache->delete($key);
+        if ($success == true) {
+          return true;
+        }
+
+        usleep(self::OPERATION_TIMEOUT_AMOUNT_MICRO_SECONDS);
       }
+
 
       return false;
 

src/Zynga/Framework/Lockable/Cache/V1/Driver/Caching.hh

@@ -17,7 +17,11 @@ use \Exception;
 class Caching extends FactoryDriverBase implements DriverInterface {
   private DriverConfigInterface $_config;
   private Map<string, LockPayloadInterface> $_locks;
-
+
+  // Lock operation would try a max of 200 times and sleep for 10000 in between
+  const int LOCK_ATTEMPTS_MAX = 200; // 200 * 10000 = 2s max wait time.
+  const int LOCK_TIMEOUT_AMOUNT_MICRO_SECONDS = 10000;
+
   public function __construct(DriverConfigInterface $config) {
     $this->_config = $config;
     $this->_locks = Map {};
@@ -109,18 +113,27 @@ class Caching extends FactoryDriverBase implements DriverInterface {
         $this->_locks->remove($lockKey);
 
       }
-
+      
       $lockCache = $this->getConfig()->getLockCache();
-
       $lockPayload = $this->getConfig()->getPayloadObject();
-
-      $addResult = $lockCache->add($lockPayload, $lockKey);
-
-      if ($addResult === true) {
-        $this->_locks->set($lockKey, $lockPayload);
-        return true;
+
+      $startTime = microtime(true);
+      $lockRetryCount = 0;
+
+      for ( $lockRetryCount = 0; $lockRetryCount < self::LOCK_ATTEMPTS_MAX; $lockRetryCount++ ) {
+        $lockPayload->setLockEstablishment(time());
+        $lockPayload->setLockRetryCount($lockRetryCount);
+        $addResult = $lockCache->add($lockPayload, $lockKey);
+        if ($addResult === true) {
+          $this->_locks->set($lockKey, $lockPayload);
+          return true;
+        }
+
+        $lockRetryCount++;
+        // if failed to get a lock, sleep and try again
+        usleep(self::LOCK_TIMEOUT_AMOUNT_MICRO_SECONDS);
       }
-
+      
       return false;
 
     } catch (Exception $e) {

src/Zynga/Framework/Lockable/Cache/V1/Driver/CachingTest.hh

@@ -48,7 +48,33 @@ class CachingTest extends TestCase {
     return true;
 
   }
+
+  public function testLockRetryLogic(): void {
+    $pgDataCache = LockableCacheFactory::factory(
+      LockableCacheDriverInterface::class,
+      'PgDataTest',
+    );
+
+    $mockId = '123456';
+    $mockTextValue = 'this-is-a-pgdata-text-value-'.mt_rand();
+
+    $inv = new InventoryModel();
 
+    $pgMock = new PgDataExample($inv);
+    $pgMock->id->set($mockId);
+    $pgMock->name->set($mockTextValue);
+
+    $this->assertTrue($pgDataCache->lock($pgMock));
+
+    $this->assertTrue($pgDataCache->isLockedByMyThread($pgMock));
+
+    // Uncomment this line and run this test in another thread to test the lock retry logic
+    // sleep(2);
+
+    $pgDataCache->unlock($pgMock);
+
+  }
+
   public function testIsLockedByMyThread(): void {
     $pgDataCache = LockableCacheFactory::factory(
       LockableCacheDriverInterface::class,

src/Zynga/Framework/Lockable/Cache/V1/Interfaces/LockPayloadInterface.hh

@@ -7,6 +7,8 @@ use Zynga\Framework\StorableObject\V1\Interfaces\StorableObjectInterface;
 interface LockPayloadInterface extends StorableObjectInterface {
   public function setLockEstablishment(int $ts): bool;
   public function getLockEstablishment(): int;
+  public function setLockRetryCount(int $count): bool;
+  public function getLockRetryCount(): int;
   public function setBacktrace(string $backtrace): bool;
   public function getBacktrace(): string;
   public function isLockStillValid(int $lockTTL): bool;

src/Zynga/Framework/Lockable/Cache/V1/LockPayload.hh

@@ -9,12 +9,16 @@ use Zynga\Framework\Lockable\Cache\V1\Interfaces\LockPayloadInterface;
 
 class LockPayload extends Base implements LockPayloadInterface {
   public UInt64Box $lockEstablishment;
+  public UInt64Box $lockRetryCount;
   public StringBox $backtrace;
 
   public function __construct() {
 
     $this->lockEstablishment = new UInt64Box();
     $this->lockEstablishment->setIsRequired(true);
+
+    $this->lockRetryCount = new UInt64Box();
+    $this->lockRetryCount->setDefaultValue(0);
 
     $this->backtrace = new StringBox();
     $this->backtrace->setIsRequired(true);
@@ -30,6 +34,13 @@ class LockPayload extends Base implements LockPayloadInterface {
   public function getLockEstablishment(): int {
     return $this->lockEstablishment->get();
   }
+
+  public function setLockRetryCount(int $count): bool {
+    return $this->lockRetryCount->set($count);
+  }
+  public function getLockRetryCount(): int {
+    return $this->lockRetryCount->get();
+  }
 
   public function setBacktrace(string $backtrace): bool {
     return $this->backtrace->set($backtrace);

src/Zynga/Framework/Lockable/Cache/V1/LockPayloadTest.hh

@@ -10,7 +10,9 @@ class LockPayloadTest extends TestCase {
   public function testIsLockStillValid_Success(): void {
     $obj = new LockPayload();
     $obj->setLockEstablishment(time());
+    $obj->setLockRetryCount(0);
     $this->assertTrue($obj->isLockStillValid(300));
+    $this->assertTrue($obj->getLockRetryCount() === 0);
   }
 
   public function testIsLockStillValid_Failure(): void {