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

Signed-off-by: Tanuja Shinde <[email protected]>

Author: Tanuja Shinde

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
+from datetime import datetime, timedelta
 
 from cortx.utils.conf_store.error import ConfError
 from cortx.utils.conf_store.conf_cache import ConfCache
@@ -279,40 +278,26 @@ def lock(self, index: str, **kwargs):
             raise ConfError(errno.EINVAL, "config index %s is not loaded",
                 index)
 
-        self.timeout = const.DEFAULT_LOCK_TIMEOUT
+        self.duration = const.DEFAULT_LOCK_DURATION
         self.lock_owner = self.default_owner
         self.lock_key = const.LOCK_KEY
-        allowed_keys = { 'lock_key', 'lock_owner', 'timeout' }
+        allowed_keys = { 'lock_key', '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)
 
-        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)
-
-        return self._get_lock_owner(index, lock_key) == lock_owner
+        rc = False
+        if not self.test_lock(index, lock_owner=self.lock_owner):
+            self.set(index, const.LOCK_OWNER_KEY % self.lock_key, self.lock_owner)
+            lock_end_time = datetime.now() + timedelta(seconds=self.duration)
+            self.set(index, const.LOCK_END_TIME_KEY % self.lock_key, str(lock_end_time))
+            rc = True
+        return rc
 
     def unlock(self, index: str, **kwargs):
         """Release config lock."""
@@ -336,24 +321,37 @@ def unlock(self, index: str, **kwargs):
 
             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_lock_owner(index, self.lock_key) == self.lock_owner or self.force:
+            self.delete(index, const.LOCK_OWNER_KEY % self.lock_key)
+            self.delete(index, const.LOCK_END_TIME_KEY % self.lock_key)
+            rc = True
+        return rc
 
     def test_lock(self, index: str, **kwargs):
         """Check whether lock is acquired on the config."""
         if index not in self._cache.keys():
             raise ConfError(errno.EINVAL, "config index %s is not loaded",
                 index)
-        allowed_keys = { 'lock_key' }
+        allowed_keys = { 'lock_key' , 'lock_owner'}
         self.lock_key = const.LOCK_KEY
+        self.lock_owner = self.default_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
+        if self._get_lock_owner(index, self.lock_key) in [None, "", self.lock_owner]:
+            return False
+
+        lock_end_time = self.get(index, const.LOCK_END_TIME_KEY % self.lock_key)
+        if lock_end_time in [None, ""]:
+            return False
+        lock_end_time = datetime.strptime(lock_end_time, "%Y-%m-%d %H:%M:%S.%f")
+        if lock_end_time < datetime.now():
+            return False
+        return True
 
     def _get_lock_owner(self, index: str, lock_key: str):
         """Get owner of the config lock."""
@@ -473,7 +471,7 @@ def lock(index: str, **kwargs):
         :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 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.
@@ -500,8 +498,9 @@ 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 lock_owner(optional, default=machine_id): Identity of owner who is sending test lock request.
 
-        :return: True if lock is acquired by somone else False
+        :return: True if lock is acquired by someone else False
         """
         return Conf._conf.test_lock(index, **kwargs)
 

py-utils/src/utils/const.py

@@ -91,7 +91,7 @@
 
 # Confstore lock keys
 LOCK_KEY = "cortx>gconf>lock"
-LOCK_TIME_KEY = "%s>time"
+LOCK_END_TIME_KEY = "%s>end_time"
 LOCK_OWNER_KEY = "%s>owner"
-DEFAULT_LOCK_TIMEOUT = 0
+DEFAULT_LOCK_DURATION = 10
 DEFAULT_RETRY_DELAY = 1