Revert "Delete blob store lock paths on unlock"

This reverts commit 6768cf939724c6858ec98cfc45dd330fde488745.

In practice, there can be many actors (ie, schedulers) locking and
unlocking a blob near-simultaneously.  This patch added only one
retry which means it can really only handle a race condition with
one other actor.

Rather than fixing this with infinite retries, revert this patch
and rely on the existing use- and time-based blobstore lock cleanup.
That is slower but does have the advantage of not constantly deleting
and re-creating lock paths for busy nodes.

Change-Id: Idb56678f24f54a790505db03fc0e57f8aeb375e8

zuul/zk/blob_store.py

@@ -131,8 +131,7 @@ class BlobStore:
                 SessionAwareLock(
                     self.context.client,
                     lock_path),
-                blocking=True,
-                delete_lock_path=True,
+                blocking=True
         ) as lock:
             if self._checkKey(key):
                 return key
@@ -154,13 +153,13 @@ class BlobStore:
         lock_path = self._getLockPath(key)
         if self.context.sessionIsInvalid():
             raise Exception("ZooKeeper session or lock not valid")
+        deleted = False
         try:
             with locked(
                     SessionAwareLock(
                         self.context.client,
                         lock_path),
-                    blocking=True,
-                    delete_lock_path=True,
+                    blocking=True
             ) as lock:
                 # make a new context based on the old one
                 with ZKContext(self.context.client, lock,
@@ -175,17 +174,17 @@ class BlobStore:
                     if zstat.last_modified_transaction_id < ltime:
                         self._retry(locked_context, self.context.client.delete,
                                     path, recursive=True)
+                        deleted = True
         except NoNodeError:
             raise KeyError(key)
-
-    def _deleteLock(self, key):
-        try:
-            lock_path = self._getLockPath(key)
-            self._retry(self.context, self.context.client.delete,
-                        lock_path)
-        except Exception:
-            self.context.log.exception(
-                "Error deleting lock path %s:", lock_path)
+        if deleted:
+            try:
+                lock_path = self._getLockPath(key)
+                self._retry(self.context, self.context.client.delete,
+                            lock_path)
+            except Exception:
+                self.context.log.exception(
+                    "Error deleting lock path %s:", lock_path)
 
     def __iter__(self):
         try:

zuul/zk/locks.py

@@ -16,7 +16,7 @@ import logging
 from contextlib import contextmanager
 from urllib.parse import quote_plus
 
-from kazoo.exceptions import NoNodeError, NotEmptyError
+from kazoo.exceptions import NoNodeError
 from kazoo.protocol.states import KazooState
 from kazoo.recipe.lock import Lock, ReadLock, WriteLock
 
@@ -108,7 +108,7 @@ class SessionAwareReadLock(SessionAwareMixin, ReadLock):
 
 
 @contextmanager
-def locked(lock, blocking=True, timeout=None, delete_lock_path=False):
+def locked(lock, blocking=True, timeout=None):
     try:
         if not lock.acquire(blocking=blocking, timeout=timeout):
             raise LockException(f"Failed to acquire lock {lock}")
@@ -123,16 +123,6 @@ def locked(lock, blocking=True, timeout=None, delete_lock_path=False):
     finally:
         try:
             lock.release()
-            if delete_lock_path:
-                try:
-                    lock.client.delete(lock.path)
-                except NoNodeError:
-                    pass
-                except NotEmptyError:
-                    pass
-                except Exception:
-                    log = logging.getLogger("zuul.zk.locks")
-                    log.exception("Failed to delete lock path %s", lock.path)
         except Exception:
             log = logging.getLogger("zuul.zk.locks")
             log.exception("Failed to release lock %s", lock)