From 7f93570a6d0a9dbc12fdf958ffb488a3544bf6f1 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Mon, 4 May 2009 22:51:04 +0200
Subject: [PATCH] Implement lock names for debugging purposes
This patch adds lock names to SharedLocks and LockSets, that can be used
later for displaying the actual locks being held/used in places where we
only have the lock, and not the entire context of the locking operation.
Since I realized that the production code doesn't call LockSet with the
proper members= syntax, but directly as positional parameters, I've
converted this (and the arguments to GlobalLockManager) into positional
arguments.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
---
lib/config.py | 4 +-
lib/jqueue.py | 4 +-
lib/locking.py | 71 ++++++++++++++++++++++-----------
test/ganeti.locking_unittest.py | 26 ++++++------
4 files changed, 64 insertions(+), 41 deletions(-)
diff --git a/lib/config.py b/lib/config.py
index c78d3f269..16acc107f 100644
--- a/lib/config.py
+++ b/lib/config.py
@@ -1,7 +1,7 @@
#
#
-# Copyright (C) 2006, 2007 Google Inc.
+# Copyright (C) 2006, 2007, 2008, 2009, 2010 Google Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -47,7 +47,7 @@ from ganeti import uidpool
from ganeti import netutils
-_config_lock = locking.SharedLock()
+_config_lock = locking.SharedLock("ConfigWriter")
# job id used for resource management at config upgrade time
_UPGRADE_CONFIG_JID = "jid-cfg-upgrade"
diff --git a/lib/jqueue.py b/lib/jqueue.py
index 1a9d781a5..110109048 100644
--- a/lib/jqueue.py
+++ b/lib/jqueue.py
@@ -1,7 +1,7 @@
#
#
-# Copyright (C) 2006, 2007, 2008 Google Inc.
+# Copyright (C) 2006, 2007, 2008, 2009, 2010 Google Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -855,7 +855,7 @@ class JobQueue(object):
# shared mode, including itself. In order not to acquire it at all
# concurrency must be guaranteed with all code acquiring it in shared mode
# and all code acquiring it exclusively.
- self._lock = locking.SharedLock()
+ self._lock = locking.SharedLock("JobQueue")
self.acquire = self._lock.acquire
self.release = self._lock.release
diff --git a/lib/locking.py b/lib/locking.py
index 0b419c71a..59ab7db47 100644
--- a/lib/locking.py
+++ b/lib/locking.py
@@ -1,7 +1,7 @@
#
#
-# Copyright (C) 2006, 2007 Google Inc.
+# Copyright (C) 2006, 2007, 2008, 2009, 2010 Google Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -404,6 +404,9 @@ class SharedLock(object):
the shared lock in the order they queued for it, just that they will
eventually do so.
+ @type name: string
+ @ivar name: the name of the lock
+
"""
__slots__ = [
"__active_shr_c",
@@ -413,16 +416,21 @@ class SharedLock(object):
"__lock",
"__pending",
"__shr",
+ "name",
]
__condition_class = PipeCondition
- def __init__(self):
+ def __init__(self, name):
"""Construct a new SharedLock.
+ @param name: the name of the lock
+
"""
object.__init__(self)
+ self.name = name
+
# Internal lock
self.__lock = threading.Lock()
@@ -445,7 +453,7 @@ class SharedLock(object):
"""
if self.__deleted:
- raise errors.LockError("Deleted lock")
+ raise errors.LockError("Deleted lock %s" % self.name)
def __is_sharer(self):
"""Is the current thread sharing the lock at this time?
@@ -537,7 +545,8 @@ class SharedLock(object):
self.__check_deleted()
# We cannot acquire the lock if we already have it
- assert not self.__is_owned(), "double acquire() on a non-recursive lock"
+ assert not self.__is_owned(), ("double acquire() on a non-recursive lock"
+ " %s" % self.name)
# Check whether someone else holds the lock or there are pending acquires.
if not self.__pending and self.__can_acquire(shared):
@@ -700,24 +709,29 @@ class LockSet:
All the locks needed in the same set must be acquired together, though.
+ @type name: string
+ @ivar name: the name of the lockset
+
"""
- def __init__(self, members=None):
+ def __init__(self, members, name):
"""Constructs a new LockSet.
@type members: list of strings
@param members: initial members of the set
"""
+ assert members is not None, "members parameter is not a list"
+ self.name = name
+
# Used internally to guarantee coherency.
- self.__lock = SharedLock()
+ self.__lock = SharedLock(name)
# The lockdict indexes the relationship name -> lock
# The order-of-locking is implied by the alphabetical order of names
self.__lockdict = {}
- if members is not None:
- for name in members:
- self.__lockdict[name] = SharedLock()
+ for mname in members:
+ self.__lockdict[mname] = SharedLock("%s/%s" % (name, mname))
# The owner dict contains the set of locks each thread owns. For
# performance each thread can access its own key without a global lock on
@@ -824,7 +838,8 @@ class LockSet:
assert timeout is None or timeout >= 0.0
# Check we don't already own locks at this level
- assert not self._is_owned(), "Cannot acquire locks in the same set twice"
+ assert not self._is_owned(), ("Cannot acquire locks in the same set twice"
+ " (lockset %s)" % self.name)
# We need to keep track of how long we spent waiting for a lock. The
# timeout passed to this function is over all lock acquires.
@@ -894,7 +909,8 @@ class LockSet:
# element is not there anymore.
continue
- raise errors.LockError("Non-existing lock in set (%s)" % lname)
+ raise errors.LockError("Non-existing lock %s in set %s" %
+ (lname, self.name))
acquire_list.append((lname, lock))
@@ -925,14 +941,16 @@ class LockSet:
# particular element is not there anymore.
continue
- raise errors.LockError("Non-existing lock in set (%s)" % lname)
+ raise errors.LockError("Non-existing lock %s in set %s" %
+ (lname, self.name))
if not acq_success:
# Couldn't get lock or timeout occurred
if timeout is None:
# This shouldn't happen as SharedLock.acquire(timeout=None) is
# blocking.
- raise errors.LockError("Failed to get lock %s" % lname)
+ raise errors.LockError("Failed to get lock %s (set %s)" %
+ (lname, self.name))
raise _AcquireTimeout()
@@ -967,7 +985,8 @@ class LockSet:
(defaults to all the locks acquired at that level).
"""
- assert self._is_owned(), "release() on lock set while not owner"
+ assert self._is_owned(), ("release() on lock set %s while not owner" %
+ self.name)
# Support passing in a single resource to release rather than many
if isinstance(names, basestring):
@@ -978,8 +997,8 @@ class LockSet:
else:
names = set(names)
assert self._list_owned().issuperset(names), (
- "release() on unheld resources %s" %
- names.difference(self._list_owned()))
+ "release() on unheld resources %s (set %s)" %
+ (names.difference(self._list_owned()), self.name))
# First of all let's release the "all elements" lock, if set.
# After this 'add' can work again
@@ -1006,7 +1025,8 @@ class LockSet:
"""
# Check we don't already own locks at this level
assert not self._is_owned() or self.__lock._is_owned(shared=0), \
- "Cannot add locks if the set is only partially owned, or shared"
+ ("Cannot add locks if the set %s is only partially owned, or shared" %
+ self.name)
# Support passing in a single resource to add rather than many
if isinstance(names, basestring):
@@ -1025,10 +1045,11 @@ class LockSet:
# This must be an explicit raise, not an assert, because assert is
# turned off when using optimization, and this can happen because of
# concurrency even if the user doesn't want it.
- raise errors.LockError("duplicate add() (%s)" % invalid_names)
+ raise errors.LockError("duplicate add(%s) on lockset %s" %
+ (invalid_names, self.name))
for lockname in names:
- lock = SharedLock()
+ lock = SharedLock("%s/%s" % (self.name, lockname))
if acquired:
lock.acquire(shared=shared)
@@ -1076,7 +1097,8 @@ class LockSet:
# to delete. The ownership must also be exclusive, but that will be checked
# by the lock itself.
assert not self._is_owned() or self._list_owned().issuperset(names), (
- "remove() on acquired lockset while not owning all elements")
+ "remove() on acquired lockset %s while not owning all elements" %
+ self.name)
removed = []
@@ -1091,7 +1113,8 @@ class LockSet:
removed.append(lname)
except (KeyError, errors.LockError):
# This cannot happen if we were already holding it, verify:
- assert not self._is_owned(), "remove failed while holding lockset"
+ assert not self._is_owned(), ("remove failed while holding lockset %s"
+ % self.name)
else:
# If no LockError was raised we are the ones who deleted the lock.
# This means we can safely remove it from lockdict, as any further or
@@ -1167,9 +1190,9 @@ class GanetiLockManager:
# The keyring contains all the locks, at their level and in the correct
# locking order.
self.__keyring = {
- LEVEL_CLUSTER: LockSet([BGL]),
- LEVEL_NODE: LockSet(nodes),
- LEVEL_INSTANCE: LockSet(instances),
+ LEVEL_CLUSTER: LockSet([BGL], "bgl lockset"),
+ LEVEL_NODE: LockSet(nodes, "nodes lockset"),
+ LEVEL_INSTANCE: LockSet(instances, "instances lockset"),
}
def _names(self, level):
diff --git a/test/ganeti.locking_unittest.py b/test/ganeti.locking_unittest.py
index 19e209cec..3eb23754a 100755
--- a/test/ganeti.locking_unittest.py
+++ b/test/ganeti.locking_unittest.py
@@ -1,7 +1,7 @@
#!/usr/bin/python
#
-# Copyright (C) 2006, 2007 Google Inc.
+# Copyright (C) 2006, 2007, 2010 Google Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -36,7 +36,7 @@ import testutils
# This is used to test the ssynchronize decorator.
# Since it's passed as input to a decorator it must be declared as a global.
-_decoratorlock = locking.SharedLock()
+_decoratorlock = locking.SharedLock("decorator lock")
#: List for looping tests
ITERATIONS = range(8)
@@ -256,7 +256,7 @@ class TestSharedLock(_ThreadedTestCase):
def setUp(self):
_ThreadedTestCase.setUp(self)
- self.sl = locking.SharedLock()
+ self.sl = locking.SharedLock("TestSharedLock")
def testSequenceAndOwnership(self):
self.assertFalse(self.sl._is_owned())
@@ -350,7 +350,7 @@ class TestSharedLock(_ThreadedTestCase):
self.sl.release()
self._waitThreads()
self.failUnlessEqual(self.done.get_nowait(), 'DEL')
- self.sl = locking.SharedLock()
+ self.sl = locking.SharedLock(self.sl.name)
@_Repeat
def testExclusiveBlocksSharer(self):
@@ -378,7 +378,7 @@ class TestSharedLock(_ThreadedTestCase):
self.sl.release()
self._waitThreads()
self.failUnlessEqual(self.done.get_nowait(), 'DEL')
- self.sl = locking.SharedLock()
+ self.sl = locking.SharedLock(self.sl.name)
@_Repeat
def testWaitingExclusiveBlocksSharer(self):
@@ -441,7 +441,7 @@ class TestSharedLock(_ThreadedTestCase):
# The threads who were pending return ERR
for _ in range(4):
self.assertEqual(self.done.get_nowait(), 'ERR')
- self.sl = locking.SharedLock()
+ self.sl = locking.SharedLock(self.sl.name)
@_Repeat
def testDeletePendingDeleteExclusiveSharers(self):
@@ -457,7 +457,7 @@ class TestSharedLock(_ThreadedTestCase):
self.assertEqual(self.done.get_nowait(), 'ERR')
self.assertEqual(self.done.get_nowait(), 'ERR')
self.assertEqual(self.done.get_nowait(), 'ERR')
- self.sl = locking.SharedLock()
+ self.sl = locking.SharedLock(self.sl.name)
@_Repeat
def testExclusiveAcquireTimeout(self):
@@ -703,7 +703,7 @@ class TestSharedLockInCondition(_ThreadedTestCase):
def setUp(self):
_ThreadedTestCase.setUp(self)
- self.sl = locking.SharedLock()
+ self.sl = locking.SharedLock("TestSharedLockInCondition")
self.setCondition()
def setCondition(self):
@@ -796,11 +796,11 @@ class TestLockSet(_ThreadedTestCase):
def _setUpLS(self):
"""Helper to (re)initialize the lock set"""
self.resources = ['one', 'two', 'three']
- self.ls = locking.LockSet(members=self.resources)
+ self.ls = locking.LockSet(self.resources, "TestLockSet")
def testResources(self):
self.assertEquals(self.ls._names(), set(self.resources))
- newls = locking.LockSet()
+ newls = locking.LockSet([], "TestLockSet.testResources")
self.assertEquals(newls._names(), set())
def testAcquireRelease(self):
@@ -1288,19 +1288,19 @@ class TestGanetiLockManager(_ThreadedTestCase):
def testInitAndResources(self):
locking.GanetiLockManager._instance = None
- self.GL = locking.GanetiLockManager()
+ self.GL = locking.GanetiLockManager([], [])
self.assertEqual(self.GL._names(locking.LEVEL_CLUSTER), set(['BGL']))
self.assertEqual(self.GL._names(locking.LEVEL_NODE), set())
self.assertEqual(self.GL._names(locking.LEVEL_INSTANCE), set())
locking.GanetiLockManager._instance = None
- self.GL = locking.GanetiLockManager(nodes=self.nodes)
+ self.GL = locking.GanetiLockManager(self.nodes, [])
self.assertEqual(self.GL._names(locking.LEVEL_CLUSTER), set(['BGL']))
self.assertEqual(self.GL._names(locking.LEVEL_NODE), set(self.nodes))
self.assertEqual(self.GL._names(locking.LEVEL_INSTANCE), set())
locking.GanetiLockManager._instance = None
- self.GL = locking.GanetiLockManager(instances=self.instances)
+ self.GL = locking.GanetiLockManager([], self.instances)
self.assertEqual(self.GL._names(locking.LEVEL_CLUSTER), set(['BGL']))
self.assertEqual(self.GL._names(locking.LEVEL_NODE), set())
self.assertEqual(self.GL._names(locking.LEVEL_INSTANCE),
--
GitLab