From e989bd0637629b0fb15976186b0941fec0f13b25 Mon Sep 17 00:00:00 2001
From: Rabi Mishra <ramishra@redhat.com>
Date: Thu, 15 Aug 2019 21:23:46 +0530
Subject: [PATCH] Add retry for DBDeadlock in credential delete

Adds oslo.db retry wrapper to delete_credential_for_user method.

Change-Id: Ib9e161411f0985785eec46c51d721ef7421ee090
Closes-Bug: #1840291
---
 keystone/credential/backends/sql.py           |  3 ++
 keystone/tests/unit/test_v3_credential.py     | 34 +++++++++++++++++++
 .../notes/bug-1840291-35af1ac7ba06e166.yaml   |  6 ++++
 3 files changed, 43 insertions(+)
 create mode 100644 releasenotes/notes/bug-1840291-35af1ac7ba06e166.yaml

diff --git a/keystone/credential/backends/sql.py b/keystone/credential/backends/sql.py
index 144307161b..e499e2a86e 100644
--- a/keystone/credential/backends/sql.py
+++ b/keystone/credential/backends/sql.py
@@ -12,6 +12,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from oslo_db import api as oslo_db_api
+
 from keystone.common import driver_hints
 from keystone.common import sql
 from keystone.credential.backends import base
@@ -96,6 +98,7 @@ class Credential(base.CredentialDriverBase):
             query = query.filter_by(project_id=project_id)
             query.delete()
 
+    @oslo_db_api.wrap_db_retry(retry_on_deadlock=True)
     def delete_credentials_for_user(self, user_id):
         with sql.session_for_write() as session:
             query = session.query(CredentialModel)
diff --git a/keystone/tests/unit/test_v3_credential.py b/keystone/tests/unit/test_v3_credential.py
index b5e08249ac..2538c3a105 100644
--- a/keystone/tests/unit/test_v3_credential.py
+++ b/keystone/tests/unit/test_v3_credential.py
@@ -17,6 +17,8 @@ import json
 import uuid
 
 from keystoneclient.contrib.ec2 import utils as ec2_utils
+import mock
+from oslo_db import exception as oslo_db_exception
 from six.moves import http_client
 from testtools import matchers
 
@@ -262,6 +264,38 @@ class CredentialTestCase(CredentialBaseTestCase):
             '/credentials/%(credential_id)s' % {
                 'credential_id': self.credential['id']})
 
+    def test_delete_credential_retries_on_deadlock(self):
+        patcher = mock.patch('sqlalchemy.orm.query.Query.delete',
+                             autospec=True)
+
+        class FakeDeadlock(object):
+            def __init__(self, mock_patcher):
+                self.deadlock_count = 2
+                self.mock_patcher = mock_patcher
+                self.patched = True
+
+            def __call__(self, *args, **kwargs):
+                if self.deadlock_count > 1:
+                    self.deadlock_count -= 1
+                else:
+                    self.mock_patcher.stop()
+                    self.patched = False
+                raise oslo_db_exception.DBDeadlock
+
+        sql_delete_mock = patcher.start()
+        side_effect = FakeDeadlock(patcher)
+        sql_delete_mock.side_effect = side_effect
+
+        try:
+            PROVIDERS.credential_api.delete_credentials_for_user(
+                user_id=self.user['id'])
+        finally:
+            if side_effect.patched:
+                patcher.stop()
+
+        # initial attempt + 1 retry
+        self.assertEqual(sql_delete_mock.call_count, 2)
+
     def test_create_ec2_credential(self):
         """Call ``POST /credentials`` for creating ec2 credential."""
         blob, ref = unit.new_ec2_credential(user_id=self.user['id'],
diff --git a/releasenotes/notes/bug-1840291-35af1ac7ba06e166.yaml b/releasenotes/notes/bug-1840291-35af1ac7ba06e166.yaml
new file mode 100644
index 0000000000..142f8e0b54
--- /dev/null
+++ b/releasenotes/notes/bug-1840291-35af1ac7ba06e166.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    [`bug 1840291 <https://bugs.launchpad.net/keystone/+bug/1840291>`_]
+    Adds retries for ``delete_credential_for_user`` method to avoid
+    DBDeadlocks when deleting large number of credentials concurrently.