CORTX-33673: Improvement for lock-unlock interface with lock end time…

py-utils/src/utils/conf_store/conf_store.py

@@ -16,8 +16,7 @@
 # please email [email protected] or [email protected].
 
 import errno
-from datetime import datetime
-from time import sleep
+import time
 
 from cortx.utils.conf_store.error import ConfError
 from cortx.utils.conf_store.conf_cache import ConfCache
@@ -42,8 +41,9 @@ def __init__(self, delim='>'):
         self._cache = {}
         self._callbacks = {}
         self._machine_id = self._get_machine_id()
-        self.default_owner = self.lock_owner = self._machine_id.strip() if self._machine_id else None
-        self.lock_key = const.LOCK_KEY
+        self.lock_owner = self._get_machine_id()
+        self.domain = const.DEFAULT_LOCK_DOMAIN
+        self.duration = const.DEFAULT_LOCK_DURATION
 
     @property
     def machine_id(self):
@@ -274,90 +274,109 @@ def _merge(self, dest_index, src_index, keys):
                 self._cache[dest_index].set(key, self._cache[src_index].get(key))
 
     def lock(self, index: str, **kwargs):
-        """Acquire lock on config."""
+        """
+        Attempt to acquire the config lock.
+
+        Parameters:
+        index(required): Identifier of the config.
+        domain(optional): Identity of the lock holder.
+        lock_owner(optional, default=machine_id): Unique instance of domain who is sending lock request.
+        duration(optional): Obtains the lock for the give duration in terms of seconds.
+
+        return: True if the lock was successfully acquired,
+            false if it is already acquired by someone.
+        """
         if index not in self._cache.keys():
             raise ConfError(errno.EINVAL, "config index %s is not loaded",
                 index)
 
-        self.timeout = const.DEFAULT_LOCK_TIMEOUT
-        self.lock_owner = self.default_owner
-        self.lock_key = const.LOCK_KEY
-        allowed_keys = { 'lock_key', 'lock_owner', 'timeout' }
+        allowed_keys = { 'domain', 'lock_owner', 'duration' }
         for key, value in kwargs.items():
             if key not in allowed_keys:
                 raise ConfError(errno.EINVAL, "Invalid parameter %s", key)
 
-            if key == 'timeout' and not isinstance(value, int):
+            if key == 'duration' and not isinstance(value, int):
                 raise ConfError(errno.EINVAL, "Invalid value %s for parameter %s", value, key)
 
             setattr(self, key, value)
+        rc = False
+        if self.test_lock(index, lock_owner=self.lock_owner, domain=self.domain):
+            self.set(index, const.LOCK_OWNER_PREFIX % self.domain, self.lock_owner)
+            lock_end_time = time.time() + self.duration
+            self.set(index, const.LOCK_END_TIME_PREFIX % self.domain, str(lock_end_time))
+            time.sleep(0.2)
+            if self.get(index, const.LOCK_OWNER_PREFIX % self.domain) == self.lock_owner:
+                rc = True
+        return rc
 
-        who_owner = self._get_lock_owner(index, self.lock_key)
-        if who_owner is not None:
-            return who_owner == self.lock_owner
-
-        while self.timeout > 1:
-            sleep(const.DEFAULT_RETRY_DELAY)
-            # TODO: Add condition_check scenario here
-            self.timeout -= 1
-
-        if not self._lock(index, self.lock_key, self.lock_owner):
-            self.lock_owner = self.default_owner
-            return False
-        return True
-
-    def _lock(self, index: str, lock_key: str, lock_owner: str):
-        """Acquire lock on config."""
-        locked_at = str(datetime.timestamp(datetime.now()))
-        self.set(index, const.LOCK_OWNER_KEY % lock_key, lock_owner)
-        self.set(index, const.LOCK_TIME_KEY % lock_key, locked_at)
+    def unlock(self, index: str, **kwargs):
+        """
+        Attempt to release the config lock.
 
-        return self._get_lock_owner(index, lock_key) == lock_owner
+        Parameters:
+        index(required): Identifier of the config.
+        domain(optional): Identity of the Lock Holder.
+        lock_owner(optional, default=machine_id): Unique instance of the domain who is sending  unlock request.
+        force(optional, default=False): When true, lock is forcefully released.
 
-    def unlock(self, index: str, **kwargs):
-        """Release config lock."""
+        return: True if the lock was successfully released,
+            false if it there is no lock or acquired by someone else unless force=true.
+        """
         if index not in self._cache.keys():
             raise ConfError(errno.EINVAL, "config index %s is not loaded",
                 index)
 
         self.force = False
-        self.lock_owner = self.default_owner
-        self.lock_key = const.LOCK_KEY
-        allowed_keys = { 'lock_key', 'lock_owner', 'force' }
+        allowed_keys = { 'domain', 'lock_owner', 'force' }
         for key, value in kwargs.items():
             if key not in allowed_keys:
                 raise ConfError(errno.EINVAL, "Invalid parameter %s", key)
 
             if key == 'force' and not isinstance(value, bool):
-                raise ConfError(
-                    errno.EINVAL, "Invalid value %s for parameter %s",
-                    value, key
-                    )
+                raise ConfError(errno.EINVAL, "Invalid value %s for parameter %s", value, key)
 
             setattr(self, key, value)
 
-        _is_locked = self._get_lock_owner(index, self.lock_key) == self.lock_owner
-        return self.delete(index, const.LOCK_OWNER_KEY % self.lock_key) if _is_locked \
-                or self.force else False
+        rc = False
+        if self.get(index, const.LOCK_OWNER_PREFIX % self.domain) == self.lock_owner or self.force:
+            self.delete(index, const.LOCK_OWNER_PREFIX % self.domain)
+            self.delete(index, const.LOCK_END_TIME_PREFIX % self.domain)
+            rc = True
+        return rc
 
     def test_lock(self, index: str, **kwargs):
-        """Check whether lock is acquired on the config."""
+        """
+        Check whether lock is acquired on the config.
+
+        Parameters:
+        index(required): param index: Identifier of the config.
+        domain(optional): Identity of the Lock Holder.
+        lock_owner(optional, default=machine_id): Unique instance of domain who needs to test lock.
+
+        return: True if lock can be acquired by someone else False
+        """
         if index not in self._cache.keys():
             raise ConfError(errno.EINVAL, "config index %s is not loaded",
                 index)
-        allowed_keys = { 'lock_key' }
-        self.lock_key = const.LOCK_KEY
+
+        allowed_keys = { 'domain' , 'lock_owner'}
         for key, value in kwargs.items():
             if key not in allowed_keys:
                 raise ConfError(errno.EINVAL, "Invalid parameter %s", key)
 
             setattr(self, key, value)
 
-        return False if self._get_lock_owner(index, self.lock_key) is None else True
+        current_lock_owner= self.get(index, const.LOCK_OWNER_PREFIX % self.domain)
+        if current_lock_owner in [None, "", self.lock_owner]:
+            return True
+
+        lock_end_time = self.get(index, const.LOCK_END_TIME_PREFIX % self.domain)
+        if lock_end_time in [None, ""]:
+            return True
 
-    def _get_lock_owner(self, index: str, lock_key: str):
-        """Get owner of the config lock."""
-        return self.get(index, const.LOCK_OWNER_KEY % lock_key)
+        if float(lock_end_time) < time.time():
+            return True
+        return False
 
 
 class Conf:
@@ -471,9 +490,9 @@ def lock(index: str, **kwargs):
         """
         Attempt to acquire the config lock.
         :param index(required): Identifier of the config.
-        :param lock_key(optional): Lock related key ex: conf>service>lock.
-        :param lock_owner(optional, default=machine_id): Identity of the lock holder.
-        :param timeout(optional): Time delay before attempting to acquire lock.
+        :param domain(optional): Identity of the lock holder.
+        :param lock_owner(optional, default=machine_id): Unique instance of domain who is sending lock request.
+        :param duration(optional): Obtains the lock for the give duration in terms of seconds.
 
         :return: True if the lock was successfully acquired,
             false if it is already acquired by someone.
@@ -485,8 +504,8 @@ def unlock(index: str, **kwargs):
         """
         Attempt to release the config lock.
         :param index(required): Identifier of the config.
-        :param lock_key(optional): Lock related key ex: conf>service>lock.
-        :param lock_owner(optional, default=machine_id): Identity of the lock holder.
+        :param domain(optional): Identity of the Lock Holder.
+        :param lock_owner(optional, default=machine_id): Unique instance of the domain who is sending unlock request.
         :param force(optional, default=False): When true, lock is forcefully released.
 
         :return: True if the lock was successfully released,
@@ -499,9 +518,10 @@ def test_lock(index: str, **kwargs):
         """
         Test whether Config is locked.
         :param index(required): param index: Identifier of the config.
-        :param lock_key(optional): Lock related key ex: conf>service>lock.
+        :param domain(optional): Identity of the Lock Holder.
+        :param lock_owner(optional, default=machine_id): Unique instance of domain who needs to test lock.
 
-        :return: True if lock is acquired by somone else False
+        :return: True if lock can be acquired by someone else False
         """
         return Conf._conf.test_lock(index, **kwargs)
 

py-utils/src/utils/const.py

@@ -90,8 +90,8 @@
 VERSION_UPGRADE = "UPGRADE"
 
 # Confstore lock keys
-LOCK_KEY = "cortx>gconf>lock"
-LOCK_TIME_KEY = "%s>time"
-LOCK_OWNER_KEY = "%s>owner"
-DEFAULT_LOCK_TIMEOUT = 0
+LOCK_END_TIME_PREFIX = "lock>%s>end_time"
+LOCK_OWNER_PREFIX = "lock>%s>owner"
+DEFAULT_LOCK_DURATION = 10
 DEFAULT_RETRY_DELAY = 1
+DEFAULT_LOCK_DOMAIN = "default"