diff options
| author | Zane Bitter <zbitter@redhat.com> | 2019-10-11 15:56:15 -0400 |
|---|---|---|
| committer | Zane Bitter <zbitter@redhat.com> | 2019-10-18 15:30:01 +0000 |
| commit | 0abf43535d63e294f4649acff0f5e4a6e9c37275 (patch) | |
| tree | f0ae30a2b605d040ec8170cba554a8a2b314ef70 | |
| parent | 747e4b765840cc2e5ac5d7862d8db7ee500df902 (diff) | |
| download | heat-0abf43535d63e294f4649acff0f5e4a6e9c37275.tar.gz | |
Don't use wrap_db_retry on subtransaction in resource_create_replacement()
The wrap_db_retry decorator must be used outside the outermost DB
transaction. Using it on a subtransaction means that the main
transaction gets marked for rollback but not closed, leaving the session
in the 'inactive' state.
This change ensures that the decorator is not used when we update the
previous resource from within a subtransaction as part of creating a
replacement resource, and instead the retry happens outside the main
transaction.
Change-Id: I28bfcc43b108d4d907098b2f0cf3553aab399553
Task: 36957
(cherry picked from commit 2398191be91d0dd145a4183f9a025fff1aa3727e)
| -rw-r--r-- | heat/db/sqlalchemy/api.py | 16 | ||||
| -rw-r--r-- | heat/tests/db/test_sqlalchemy_api.py | 8 |
2 files changed, 16 insertions, 8 deletions
diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py index 3966b0e3a..583e2cd78 100644 --- a/heat/db/sqlalchemy/api.py +++ b/heat/db/sqlalchemy/api.py @@ -260,6 +260,12 @@ def _add_atomic_key_to_values(values, atomic_key): retry_interval=0.5, inc_retry_interval=True) def resource_update(context, resource_id, values, atomic_key, expected_engine_id=None): + return _try_resource_update(context, resource_id, values, atomic_key, + expected_engine_id) + + +def _try_resource_update(context, resource_id, values, atomic_key, + expected_engine_id=None): session = context.session with session.begin(subtransactions=True): _add_atomic_key_to_values(values, atomic_key) @@ -448,6 +454,8 @@ def resource_create(context, values): return resource_ref +@oslo_db_api.wrap_db_retry(max_retries=3, retry_on_deadlock=True, + retry_interval=0.5, inc_retry_interval=True) def resource_create_replacement(context, existing_res_id, existing_res_values, new_res_values, @@ -458,10 +466,10 @@ def resource_create_replacement(context, new_res = resource_create(context, new_res_values) update_data = {'replaced_by': new_res.id} update_data.update(existing_res_values) - if not resource_update(context, - existing_res_id, update_data, - atomic_key, - expected_engine_id=expected_engine_id): + if not _try_resource_update(context, + existing_res_id, update_data, + atomic_key, + expected_engine_id=expected_engine_id): data = {} if 'name' in new_res_values: data['resource_name'] = new_res_values['name'] diff --git a/heat/tests/db/test_sqlalchemy_api.py b/heat/tests/db/test_sqlalchemy_api.py index 096d3b88f..77821a7e2 100644 --- a/heat/tests/db/test_sqlalchemy_api.py +++ b/heat/tests/db/test_sqlalchemy_api.py @@ -2685,8 +2685,8 @@ class DBAPIResourceReplacementTest(common.HeatTestCase): db_api.resource_update_and_save(other_ctx, orig.id, {'atomic_key': 2}) - self.patchobject(db_api, 'resource_update', - new=mock.Mock(wraps=db_api.resource_update, + self.patchobject(db_api, '_try_resource_update', + new=mock.Mock(wraps=db_api._try_resource_update, side_effect=update_atomic_key)) self.assertRaises(exception.UpdateInProgress, @@ -2728,8 +2728,8 @@ class DBAPIResourceReplacementTest(common.HeatTestCase): {'engine_id': 'a', 'atomic_key': 2}) - self.patchobject(db_api, 'resource_update', - new=mock.Mock(wraps=db_api.resource_update, + self.patchobject(db_api, '_try_resource_update', + new=mock.Mock(wraps=db_api._try_resource_update, side_effect=lock_resource)) self.assertRaises(exception.UpdateInProgress, |