From 2c1524866acb9f9ac3f50dc7d33338cfb03fd08a Mon Sep 17 00:00:00 2001 From: Stanislaw Pitucha Date: Thu, 2 Aug 2012 13:51:07 +0100 Subject: Improve external lock implementation Remove a number of limitations from the external locks. - They can be nested now - They do not need external cleanup in case of failures - They do not rely on lockfile or greenlet internal implementation New implementation is based on fcntl locks and any crashing process will drop the lock. It does not have to rely on any cleanup code or handling exceptions. Because no cleanup is needed, a number of tests have been removed. This implementation is not portable outside of POSIX/BSD/SVR4 systems. Fcntl locks should work correctly with NFS mounts. Locks are cleaned up after the tests finish running via run_tests.sh, even though it's not strictly needed. This change requires eventlet >= 0.9.17. bp improve-external-locking Change-Id: Idf5424c04645f25097733848a007b150145b0b27 --- nova/tests/test_misc.py | 16 ++--- nova/tests/test_utils.py | 176 +++-------------------------------------------- 2 files changed, 15 insertions(+), 177 deletions(-) (limited to 'nova/tests') diff --git a/nova/tests/test_misc.py b/nova/tests/test_misc.py index 87971a6a9..58d52290b 100644 --- a/nova/tests/test_misc.py +++ b/nova/tests/test_misc.py @@ -21,7 +21,6 @@ import select from eventlet import greenpool from eventlet import greenthread -import lockfile from nova import exception from nova import test @@ -107,20 +106,19 @@ class LockTestCase(test.TestCase): self.assertEqual(saved_sem_num, len(utils._semaphores), "Semaphore leak detected") - def test_nested_external_fails(self): - """We can not nest external syncs""" + def test_nested_external_works(self): + """We can nest external syncs""" + sentinel = object() @utils.synchronized('testlock1', external=True) def outer_lock(): @utils.synchronized('testlock2', external=True) def inner_lock(): - pass - inner_lock() - try: - self.assertRaises(lockfile.NotMyLock, outer_lock) - finally: - utils.cleanup_file_locks() + return sentinel + return inner_lock() + + self.assertEqual(sentinel, outer_lock()) def test_synchronized_externally(self): """We can lock across multiple processes""" diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py index 7513ca12e..898d23343 100644 --- a/nova/tests/test_utils.py +++ b/nova/tests/test_utils.py @@ -533,29 +533,22 @@ class MonkeyPatchTestCase(test.TestCase): in nova.tests.monkey_patch_example.CALLED_FUNCTION) -class TestGreenLocks(test.TestCase): +class TestFileLocks(test.TestCase): def test_concurrent_green_lock_succeeds(self): - """Verify spawn_n greenthreads with two locks run concurrently. - - This succeeds with spawn but fails with spawn_n because lockfile - gets the same thread id for both spawn_n threads. Our workaround - of using the GreenLockFile will work even if the issue is fixed. - """ + """Verify spawn_n greenthreads with two locks run concurrently.""" self.completed = False with utils.tempdir() as tmpdir: def locka(wait): - a = utils.GreenLockFile(os.path.join(tmpdir, 'a')) - a.acquire() - wait.wait() - a.release() + a = utils.InterProcessLock(os.path.join(tmpdir, 'a')) + with a: + wait.wait() self.completed = True def lockb(wait): - b = utils.GreenLockFile(os.path.join(tmpdir, 'b')) - b.acquire() - wait.wait() - b.release() + b = utils.InterProcessLock(os.path.join(tmpdir, 'b')) + with b: + wait.wait() wait1 = eventlet.event.Event() wait2 = eventlet.event.Event() @@ -569,159 +562,6 @@ class TestGreenLocks(test.TestCase): self.assertTrue(self.completed) -class TestLockCleanup(test.TestCase): - """unit tests for utils.cleanup_file_locks()""" - - def setUp(self): - super(TestLockCleanup, self).setUp() - - self.pid = os.getpid() - self.dead_pid = self._get_dead_pid() - self.tempdir = tempfile.mkdtemp() - self.flags(lock_path=self.tempdir) - self.lock_name = 'nova-testlock' - self.lock_file = os.path.join(FLAGS.lock_path, - self.lock_name + '.lock') - self.hostname = socket.gethostname() - print self.pid, self.dead_pid - try: - os.unlink(self.lock_file) - except OSError as (errno, strerror): - if errno == 2: - pass - - def tearDown(self): - shutil.rmtree(self.tempdir) - super(TestLockCleanup, self).tearDown() - - def _get_dead_pid(self): - """get a pid for a process that does not exist""" - - candidate_pid = self.pid - 1 - while os.path.exists(os.path.join('/proc', str(candidate_pid))): - candidate_pid -= 1 - if candidate_pid == 1: - return 0 - return candidate_pid - - def _get_sentinel_name(self, hostname, pid, thread='MainThread'): - return os.path.join(FLAGS.lock_path, - '%s-%s.%d' % (hostname, thread, pid)) - - def _create_sentinel(self, hostname, pid, thread='MainThread'): - name = self._get_sentinel_name(hostname, pid, thread) - open(name, 'wb').close() - return name - - def test_clean_stale_locks(self): - """verify locks for dead processes are cleaned up""" - - # create sentinels for two processes, us and a 'dead' one - # no active lock - sentinel1 = self._create_sentinel(self.hostname, self.pid) - sentinel2 = self._create_sentinel(self.hostname, self.dead_pid) - - utils.cleanup_file_locks() - - self.assertTrue(os.path.exists(sentinel1)) - self.assertFalse(os.path.exists(self.lock_file)) - self.assertFalse(os.path.exists(sentinel2)) - - os.unlink(sentinel1) - - def test_clean_stale_locks_active(self): - """verify locks for dead processes are cleaned with an active lock """ - - # create sentinels for two processes, us and a 'dead' one - # create an active lock for us - sentinel1 = self._create_sentinel(self.hostname, self.pid) - sentinel2 = self._create_sentinel(self.hostname, self.dead_pid) - os.link(sentinel1, self.lock_file) - - utils.cleanup_file_locks() - - self.assertTrue(os.path.exists(sentinel1)) - self.assertTrue(os.path.exists(self.lock_file)) - self.assertFalse(os.path.exists(sentinel2)) - - os.unlink(sentinel1) - os.unlink(self.lock_file) - - def test_clean_stale_with_threads(self): - """verify locks for multiple threads are cleaned up """ - - # create sentinels for four threads in our process, and a 'dead' - # process. no lock. - sentinel1 = self._create_sentinel(self.hostname, self.pid, 'Default-1') - sentinel2 = self._create_sentinel(self.hostname, self.pid, 'Default-2') - sentinel3 = self._create_sentinel(self.hostname, self.pid, 'Default-3') - sentinel4 = self._create_sentinel(self.hostname, self.pid, 'Default-4') - sentinel5 = self._create_sentinel(self.hostname, self.dead_pid, - 'Default-1') - - utils.cleanup_file_locks() - - self.assertTrue(os.path.exists(sentinel1)) - self.assertTrue(os.path.exists(sentinel2)) - self.assertTrue(os.path.exists(sentinel3)) - self.assertTrue(os.path.exists(sentinel4)) - self.assertFalse(os.path.exists(self.lock_file)) - self.assertFalse(os.path.exists(sentinel5)) - - os.unlink(sentinel1) - os.unlink(sentinel2) - os.unlink(sentinel3) - os.unlink(sentinel4) - - def test_clean_stale_with_threads_active(self): - """verify locks for multiple threads are cleaned up """ - - # create sentinels for four threads in our process, and a 'dead' - # process - sentinel1 = self._create_sentinel(self.hostname, self.pid, 'Default-1') - sentinel2 = self._create_sentinel(self.hostname, self.pid, 'Default-2') - sentinel3 = self._create_sentinel(self.hostname, self.pid, 'Default-3') - sentinel4 = self._create_sentinel(self.hostname, self.pid, 'Default-4') - sentinel5 = self._create_sentinel(self.hostname, self.dead_pid, - 'Default-1') - - os.link(sentinel1, self.lock_file) - - utils.cleanup_file_locks() - - self.assertTrue(os.path.exists(sentinel1)) - self.assertTrue(os.path.exists(sentinel2)) - self.assertTrue(os.path.exists(sentinel3)) - self.assertTrue(os.path.exists(sentinel4)) - self.assertTrue(os.path.exists(self.lock_file)) - self.assertFalse(os.path.exists(sentinel5)) - - os.unlink(sentinel1) - os.unlink(sentinel2) - os.unlink(sentinel3) - os.unlink(sentinel4) - os.unlink(self.lock_file) - - def test_clean_bogus_lockfiles(self): - """verify lockfiles are cleaned """ - - lock1 = os.path.join(FLAGS.lock_path, 'nova-testlock1.lock') - lock2 = os.path.join(FLAGS.lock_path, 'nova-testlock2.lock') - lock3 = os.path.join(FLAGS.lock_path, 'testlock3.lock') - - open(lock1, 'wb').close() - open(lock2, 'wb').close() - open(lock3, 'wb').close() - - utils.cleanup_file_locks() - - self.assertFalse(os.path.exists(lock1)) - self.assertFalse(os.path.exists(lock2)) - self.assertTrue(os.path.exists(lock3)) - - os.unlink(lock3) - - class AuditPeriodTest(test.TestCase): def setUp(self): -- cgit