Corrected max secs for concurrent trans retries
This was most likely meant as a max 2s delay here, not a max 2ms delay. Also includes a related change: when retries for metadata updates are attempted, make sure we do not have a stale value of the atomic_key (otherwise we'll just inevitably hit the ConcurrentTransaction issue). Co-Authored-By: Crag Wolfe <cwolfe@redhat.com> Partial-Bug: #1651768 Change-Id: Ie56e0e4ff93633db1f4752859d2b2a9506922911
This commit is contained in:
parent
5529700be2
commit
e37d9fab8f
@ -100,7 +100,10 @@ class SoftwareConfigService(object):
|
|||||||
def _push_metadata_software_deployments(
|
def _push_metadata_software_deployments(
|
||||||
self, cnxt, server_id, stack_user_project_id):
|
self, cnxt, server_id, stack_user_project_id):
|
||||||
rs = db_api.resource_get_by_physical_resource_id(cnxt, server_id)
|
rs = db_api.resource_get_by_physical_resource_id(cnxt, server_id)
|
||||||
if not rs or rs.action == resource.Resource.DELETE:
|
if not rs:
|
||||||
|
return
|
||||||
|
cnxt.session.refresh(rs)
|
||||||
|
if rs.action == resource.Resource.DELETE:
|
||||||
return
|
return
|
||||||
deployments = self.metadata_software_deployments(cnxt, server_id)
|
deployments = self.metadata_software_deployments(cnxt, server_id)
|
||||||
md = rs.rsrc_metadata or {}
|
md = rs.rsrc_metadata or {}
|
||||||
|
@ -41,7 +41,7 @@ LOG = logging.getLogger(__name__)
|
|||||||
def retry_on_conflict(func):
|
def retry_on_conflict(func):
|
||||||
wrapper = tenacity.retry(
|
wrapper = tenacity.retry(
|
||||||
stop=tenacity.stop_after_attempt(11),
|
stop=tenacity.stop_after_attempt(11),
|
||||||
wait=tenacity.wait_random(max=0.002),
|
wait=tenacity.wait_random(max=2),
|
||||||
retry=tenacity.retry_if_exception_type(
|
retry=tenacity.retry_if_exception_type(
|
||||||
exception.ConcurrentTransaction),
|
exception.ConcurrentTransaction),
|
||||||
reraise=True)
|
reraise=True)
|
||||||
|
@ -649,8 +649,9 @@ class SoftwareConfigServiceTest(common.HeatTestCase):
|
|||||||
'deployments': {'deploy': 'this'}
|
'deployments': {'deploy': 'this'}
|
||||||
}
|
}
|
||||||
|
|
||||||
self.engine.software_config._push_metadata_software_deployments(
|
with mock.patch.object(self.ctx.session, 'refresh'):
|
||||||
self.ctx, '1234', None)
|
self.engine.software_config._push_metadata_software_deployments(
|
||||||
|
self.ctx, '1234', None)
|
||||||
res_upd.assert_called_once_with(
|
res_upd.assert_called_once_with(
|
||||||
self.ctx, '1234', {'rsrc_metadata': result_metadata}, 1)
|
self.ctx, '1234', {'rsrc_metadata': result_metadata}, 1)
|
||||||
put.side_effect = Exception('Unexpected requests.put')
|
put.side_effect = Exception('Unexpected requests.put')
|
||||||
@ -674,12 +675,14 @@ class SoftwareConfigServiceTest(common.HeatTestCase):
|
|||||||
deployments = {'deploy': 'this'}
|
deployments = {'deploy': 'this'}
|
||||||
md_sd.return_value = deployments
|
md_sd.return_value = deployments
|
||||||
|
|
||||||
self.assertRaises(
|
with mock.patch.object(self.ctx.session, 'refresh'):
|
||||||
exception.ConcurrentTransaction,
|
f = self.engine.software_config._push_metadata_software_deployments
|
||||||
self.engine.software_config._push_metadata_software_deployments,
|
self.assertRaises(
|
||||||
self.ctx,
|
exception.ConcurrentTransaction,
|
||||||
'1234',
|
f,
|
||||||
None)
|
self.ctx,
|
||||||
|
'1234',
|
||||||
|
None)
|
||||||
# retry ten times then the final failure
|
# retry ten times then the final failure
|
||||||
self.assertEqual(11, res_upd.call_count)
|
self.assertEqual(11, res_upd.call_count)
|
||||||
put.assert_not_called()
|
put.assert_not_called()
|
||||||
@ -709,9 +712,9 @@ class SoftwareConfigServiceTest(common.HeatTestCase):
|
|||||||
'original': 'metadata',
|
'original': 'metadata',
|
||||||
'deployments': {'deploy': 'this'}
|
'deployments': {'deploy': 'this'}
|
||||||
}
|
}
|
||||||
|
with mock.patch.object(self.ctx.session, 'refresh'):
|
||||||
self.engine.software_config._push_metadata_software_deployments(
|
self.engine.software_config._push_metadata_software_deployments(
|
||||||
self.ctx, '1234', None)
|
self.ctx, '1234', None)
|
||||||
res_upd.assert_called_once_with(
|
res_upd.assert_called_once_with(
|
||||||
self.ctx, '1234', {'rsrc_metadata': result_metadata}, 1)
|
self.ctx, '1234', {'rsrc_metadata': result_metadata}, 1)
|
||||||
|
|
||||||
@ -748,8 +751,9 @@ class SoftwareConfigServiceTest(common.HeatTestCase):
|
|||||||
'deployments': {'deploy': 'this'}
|
'deployments': {'deploy': 'this'}
|
||||||
}
|
}
|
||||||
|
|
||||||
self.engine.software_config._push_metadata_software_deployments(
|
with mock.patch.object(self.ctx.session, 'refresh'):
|
||||||
self.ctx, '1234', 'project1')
|
self.engine.software_config._push_metadata_software_deployments(
|
||||||
|
self.ctx, '1234', 'project1')
|
||||||
res_upd.assert_called_once_with(
|
res_upd.assert_called_once_with(
|
||||||
self.ctx, '1234', {'rsrc_metadata': result_metadata}, 1)
|
self.ctx, '1234', {'rsrc_metadata': result_metadata}, 1)
|
||||||
|
|
||||||
@ -917,8 +921,9 @@ class SoftwareConfigServiceTest(common.HeatTestCase):
|
|||||||
zaqar_client.queue.return_value = queue
|
zaqar_client.queue.return_value = queue
|
||||||
queue.pop.return_value = [mock.Mock(body='ok')]
|
queue.pop.return_value = [mock.Mock(body='ok')]
|
||||||
|
|
||||||
deployment = self._create_software_deployment(
|
with mock.patch.object(self.ctx.session, 'refresh'):
|
||||||
status='IN_PROGRESS', config_id=config['id'])
|
deployment = self._create_software_deployment(
|
||||||
|
status='IN_PROGRESS', config_id=config['id'])
|
||||||
|
|
||||||
deployment_id = deployment['id']
|
deployment_id = deployment['id']
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
|
Loading…
Reference in New Issue
Block a user