From 45722e6cb448eb7a469d9910e495925842b58fc1 Mon Sep 17 00:00:00 2001
From: Rabi Mishra <ramishra@redhat.com>
Date: Sat, 12 Mar 2016 12:17:52 +0530
Subject: [PATCH] Add retries to avoid dberror for user_creds_delete

Add retries for user_creds_delete.

Also adds cascade_backrefs=False to the relationship
between user_creds and stack.

Change-Id: I0e242c008d3368d30708b52f83f998b77aff5f35
Closes-Bug: #1545577
---
 heat/db/sqlalchemy/api.py            |  2 ++
 heat/db/sqlalchemy/models.py         |  3 ++-
 heat/db/sqlalchemy/utils.py          | 10 ++++++++++
 heat/tests/db/test_sqlalchemy_api.py | 23 +++++++++++++++++++++++
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py
index e9f5752f9e..dc4112cdda 100644
--- a/heat/db/sqlalchemy/api.py
+++ b/heat/db/sqlalchemy/api.py
@@ -36,6 +36,7 @@ from heat.common.i18n import _
 from heat.db.sqlalchemy import filters as db_filters
 from heat.db.sqlalchemy import migration
 from heat.db.sqlalchemy import models
+from heat.db.sqlalchemy import utils as db_utils
 from heat.rpc import api as rpc_api
 
 CONF = cfg.CONF
@@ -685,6 +686,7 @@ def user_creds_get(user_creds_id):
     return result
 
 
+@db_utils.retry_on_stale_data_error
 def user_creds_delete(context, user_creds_id):
     creds = model_query(context, models.UserCreds).get(user_creds_id)
     if not creds:
diff --git a/heat/db/sqlalchemy/models.py b/heat/db/sqlalchemy/models.py
index d45fced9b8..76e41d1d4f 100644
--- a/heat/db/sqlalchemy/models.py
+++ b/heat/db/sqlalchemy/models.py
@@ -215,7 +215,8 @@ class UserCreds(BASE, HeatBase):
     tenant_id = sqlalchemy.Column(sqlalchemy.String(256))
     trust_id = sqlalchemy.Column(sqlalchemy.String(255))
     trustor_user_id = sqlalchemy.Column(sqlalchemy.String(64))
-    stack = relationship(Stack, backref=backref('user_creds'))
+    stack = relationship(Stack, backref=backref('user_creds'),
+                         cascade_backrefs=False)
 
 
 class Event(BASE, HeatBase):
diff --git a/heat/db/sqlalchemy/utils.py b/heat/db/sqlalchemy/utils.py
index dae156dcb6..f6f8fc8a9f 100644
--- a/heat/db/sqlalchemy/utils.py
+++ b/heat/db/sqlalchemy/utils.py
@@ -13,7 +13,9 @@
 
 # SQLAlchemy helper functions
 
+import retrying
 import sqlalchemy
+from sqlalchemy.orm import exc
 
 
 def clone_table(name, parent, meta, newcols=None, ignorecols=None,
@@ -86,3 +88,11 @@ def migrate_data(migrate_engine,
     table.drop()
 
     new_table.rename(table_name)
+
+
+def retry_on_stale_data_error(func):
+    def is_staledata_error(ex):
+        return isinstance(ex, exc.StaleDataError)
+    wrapper = retrying.retry(stop_max_attempt_number=3,
+                             retry_on_exception=is_staledata_error)
+    return wrapper(func)
diff --git a/heat/tests/db/test_sqlalchemy_api.py b/heat/tests/db/test_sqlalchemy_api.py
index c1fc15eff6..39f6f04521 100644
--- a/heat/tests/db/test_sqlalchemy_api.py
+++ b/heat/tests/db/test_sqlalchemy_api.py
@@ -22,6 +22,9 @@ from oslo_config import cfg
 from oslo_db import exception as db_exception
 from oslo_utils import timeutils
 import six
+from sqlalchemy.orm import exc
+from sqlalchemy.orm import session
+
 
 from heat.common import context
 from heat.common import exception
@@ -1573,12 +1576,32 @@ class DBAPIUserCredsTest(common.HeatTestCase):
         db_api.user_creds_delete(self.ctx, user_creds['id'])
         creds = db_api.user_creds_get(user_creds['id'])
         self.assertIsNone(creds)
+        mock_delete = self.patchobject(session.Session, 'delete')
         err = self.assertRaises(
             exception.NotFound, db_api.user_creds_delete,
             self.ctx, user_creds['id'])
         exp_msg = ('Attempt to delete user creds with id '
                    '%s that does not exist' % user_creds['id'])
         self.assertIn(exp_msg, six.text_type(err))
+        self.assertEqual(0, mock_delete.call_count)
+
+    def test_user_creds_delete_retries(self):
+        mock_delete = self.patchobject(session.Session, 'delete')
+        # returns StaleDataErrors, so we try delete 3 times
+        mock_delete.side_effect = [exc.StaleDataError,
+                                   exc.StaleDataError,
+                                   None]
+        user_creds = create_user_creds(self.ctx)
+        self.assertIsNotNone(user_creds['id'])
+        self.assertIsNone(
+            db_api.user_creds_delete(self.ctx, user_creds['id']))
+        self.assertEqual(3, mock_delete.call_count)
+
+        # returns other errors, so we try delete once
+        mock_delete.side_effect = [exc.UnmappedError]
+        self.assertRaises(exc.UnmappedError, db_api.user_creds_delete,
+                          self.ctx, user_creds['id'])
+        self.assertEqual(4, mock_delete.call_count)
 
 
 class DBAPIStackTagTest(common.HeatTestCase):