Merge "Refactor deferral of stack state persistence"

This commit is contained in:
Zuul 2018-08-09 12:56:24 +00:00 committed by Gerrit Code Review
commit 8115054173
4 changed files with 43 additions and 40 deletions

View File

@ -180,10 +180,7 @@ class ThreadGroupManager(object):
Persist the stack state to COMPLETE and FAILED close to
releasing the lock to avoid race conditions.
"""
if (stack is not None and stack.status != stack.IN_PROGRESS
and stack.action not in (stack.DELETE,
stack.ROLLBACK,
stack.UPDATE)):
if stack is not None and stack.defer_state_persist():
stack.persist_state_and_release_lock(lock.engine_id)
notify = kwargs.get('notify')

View File

@ -957,6 +957,29 @@ class Stack(collections.Mapping):
self.env.get_event_sinks(),
ev.as_dict())
def defer_state_persist(self):
"""Return whether to defer persisting the state.
If persistence is deferred, the new state will not be written to the
database until the stack lock is released (by calling
persist_state_and_release_lock()). This prevents races in the legacy
path where an observer sees the stack COMPLETE but an engine still
holds the lock.
"""
if self.status == self.IN_PROGRESS:
# Always persist IN_PROGRESS immediately
return False
if (self.convergence and
self.action in {self.UPDATE, self.DELETE, self.CREATE,
self.ADOPT, self.ROLLBACK, self.RESTORE}):
# These operations do not use the stack lock in convergence, so
# never defer.
return False
return self.action not in {self.UPDATE, self.DELETE, self.ROLLBACK,
self.RESTORE}
@profiler.trace('Stack.state_set', hide_args=False)
def state_set(self, action, status, reason):
"""Update the stack state."""
@ -971,13 +994,9 @@ class Stack(collections.Mapping):
self.status_reason = reason
self._log_status()
if self.convergence and action in (
self.UPDATE, self.DELETE, self.CREATE,
self.ADOPT, self.ROLLBACK, self.RESTORE):
# if convergence and stack operation is create/update/rollback/
# delete, stack lock is not used, hence persist state
if not self.defer_state_persist():
updated = self._persist_state()
if not updated:
if self.convergence and not updated:
LOG.info("Stack %(name)s traversal %(trvsl_id)s no longer "
"active; not setting state to %(action)s_%(status)s",
{'name': self.name,
@ -985,13 +1004,6 @@ class Stack(collections.Mapping):
'action': action, 'status': status})
return updated
# Persist state to db only if status == IN_PROGRESS
# or action == UPDATE/DELETE/ROLLBACK. Else, it would
# be done before releasing the stack lock.
if status == self.IN_PROGRESS or action in (
self.UPDATE, self.DELETE, self.ROLLBACK, self.RESTORE):
self._persist_state()
def _log_status(self):
LOG.info('Stack %(action)s %(status)s (%(name)s): %(reason)s',
{'action': self.action,
@ -1148,8 +1160,10 @@ class Stack(collections.Mapping):
'Failed stack pre-ops: %s' % six.text_type(e))
if callable(post_func):
post_func()
# No need to call notify.signal(), because persistence of the
# state is always deferred here.
if notify is not None:
# No need to call notify.signal(), because persistence of the
# state is always deferred here.
assert self.defer_state_persist()
return
self.state_set(action, self.IN_PROGRESS,
'Stack %s started' % action)

View File

@ -587,38 +587,31 @@ class TestConvgStackStateSet(common.HeatTestCase):
def test_state_set_stack_suspend(self, mock_ps):
mock_ps.return_value = 'updated'
ret_val = self.stack.state_set(
self.stack.SUSPEND, self.stack.IN_PROGRESS, 'Suspend started')
self.stack.state_set(self.stack.SUSPEND, self.stack.IN_PROGRESS,
'Suspend started')
self.assertTrue(mock_ps.called)
# Ensure that state_set returns None for other actions in convergence
self.assertIsNone(ret_val)
mock_ps.reset_mock()
ret_val = self.stack.state_set(
self.stack.SUSPEND, self.stack.COMPLETE, 'Suspend complete')
self.stack.state_set(self.stack.SUSPEND, self.stack.COMPLETE,
'Suspend complete')
self.assertFalse(mock_ps.called)
self.assertIsNone(ret_val)
def test_state_set_stack_resume(self, mock_ps):
ret_val = self.stack.state_set(
self.stack.RESUME, self.stack.IN_PROGRESS, 'Resume started')
self.stack.state_set(self.stack.RESUME, self.stack.IN_PROGRESS,
'Resume started')
self.assertTrue(mock_ps.called)
self.assertIsNone(ret_val)
mock_ps.reset_mock()
ret_val = self.stack.state_set(self.stack.RESUME, self.stack.COMPLETE,
'Resume complete')
self.stack.state_set(self.stack.RESUME, self.stack.COMPLETE,
'Resume complete')
self.assertFalse(mock_ps.called)
self.assertIsNone(ret_val)
def test_state_set_stack_snapshot(self, mock_ps):
ret_val = self.stack.state_set(
self.stack.SNAPSHOT, self.stack.IN_PROGRESS, 'Snapshot started')
self.stack.state_set(self.stack.SNAPSHOT, self.stack.IN_PROGRESS,
'Snapshot started')
self.assertTrue(mock_ps.called)
self.assertIsNone(ret_val)
mock_ps.reset_mock()
ret_val = self.stack.state_set(
self.stack.SNAPSHOT, self.stack.COMPLETE, 'Snapshot complete')
self.stack.state_set(self.stack.SNAPSHOT, self.stack.COMPLETE,
'Snapshot complete')
self.assertFalse(mock_ps.called)
self.assertIsNone(ret_val)
def test_state_set_stack_restore(self, mock_ps):
mock_ps.return_value = 'updated'

View File

@ -3091,8 +3091,7 @@ class StackStateSetTest(common.HeatTestCase):
self.assertRaises(ValueError, self.stack.state_set,
self.action, self.status, 'test')
else:
self.assertIsNone(self.stack.state_set(self.action,
self.status, 'test'))
self.stack.state_set(self.action, self.status, 'test')
self.assertEqual((self.action, self.status), self.stack.state)
self.assertEqual('test', self.stack.status_reason)
self.assertEqual(self.persist_count, persist_state.call_count)