From cc46910f58be0c63fd8b74f25a3c0a625ee7b816 Mon Sep 17 00:00:00 2001
From: Mike Kolesnik <mkolesni@redhat.com>
Date: Tue, 23 May 2017 10:31:53 +0300
Subject: [PATCH] Change journal entry selection to optimistic locking

Journal entry selection with SELECT FOR UPDATE can cause deadlocks if
calculating dependencies, it was changed to optimistic locking to avoid this.

Change-Id: I5692b1f2e4bfe5eac3ead4b6ac32c64d5ea1f180
---
 networking_odl/db/db.py                       |  3 +-
 .../alembic_migrations/versions/EXPAND_HEAD   |  2 +-
 ...added_version_id_for_optimistic_locking.py | 36 +++++++++++++++++++
 networking_odl/db/models.py                   |  5 +++
 networking_odl/tests/unit/db/test_db.py       |  9 ++---
 5 files changed, 48 insertions(+), 7 deletions(-)
 create mode 100644 networking_odl/db/migration/alembic_migrations/versions/pike/expand/43af357fd638_added_version_id_for_optimistic_locking.py

diff --git a/networking_odl/db/db.py b/networking_odl/db/db.py
index c584a7e5d..e920b42ad 100644
--- a/networking_odl/db/db.py
+++ b/networking_odl/db/db.py
@@ -94,8 +94,7 @@ def get_oldest_pending_db_row_with_lock(session):
     with session.begin():
         row = session.query(models.OpenDaylightJournal).filter_by(
             state=odl_const.PENDING).order_by(
-            asc(models.OpenDaylightJournal.last_retried)).with_for_update(
-        ).first()
+            asc(models.OpenDaylightJournal.last_retried)).first()
         if row:
             update_db_row_state(session, row, odl_const.PROCESSING)
 
diff --git a/networking_odl/db/migration/alembic_migrations/versions/EXPAND_HEAD b/networking_odl/db/migration/alembic_migrations/versions/EXPAND_HEAD
index f795fc3f0..242701896 100644
--- a/networking_odl/db/migration/alembic_migrations/versions/EXPAND_HEAD
+++ b/networking_odl/db/migration/alembic_migrations/versions/EXPAND_HEAD
@@ -1 +1 @@
-3d560427d776
+43af357fd638
diff --git a/networking_odl/db/migration/alembic_migrations/versions/pike/expand/43af357fd638_added_version_id_for_optimistic_locking.py b/networking_odl/db/migration/alembic_migrations/versions/pike/expand/43af357fd638_added_version_id_for_optimistic_locking.py
new file mode 100644
index 000000000..02511762f
--- /dev/null
+++ b/networking_odl/db/migration/alembic_migrations/versions/pike/expand/43af357fd638_added_version_id_for_optimistic_locking.py
@@ -0,0 +1,36 @@
+# Copyright (C) 2017 Red Hat Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+"""Added version_id for optimistic locking
+
+Revision ID: 43af357fd638
+Revises: 3d560427d776
+Create Date: 2016-03-24 10:14:56.408413
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '43af357fd638'
+down_revision = '3d560427d776'
+depends_on = ('fa0c536252a5',)
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.add_column('opendaylightjournal',
+                  sa.Column('version_id', sa.Integer, server_default='0',
+                            nullable=False))
diff --git a/networking_odl/db/models.py b/networking_odl/db/models.py
index b28dd412a..094cc44c9 100644
--- a/networking_odl/db/models.py
+++ b/networking_odl/db/models.py
@@ -40,6 +40,11 @@ class OpenDaylightJournal(model_base.BASEV2):
         server_default=sa.func.now())
     last_retried = sa.Column(sa.TIMESTAMP, server_default=sa.func.now(),
                              onupdate=sa.func.now())
+    version_id = sa.Column(sa.Integer, server_default='0', nullable=False)
+
+    __mapper_args__ = {
+        'version_id_col': version_id
+    }
 
 
 class OpenDaylightMaintenance(model_base.BASEV2, model_base.HasId):
diff --git a/networking_odl/tests/unit/db/test_db.py b/networking_odl/tests/unit/db/test_db.py
index 036163f1a..89ff2faaf 100644
--- a/networking_odl/tests/unit/db/test_db.py
+++ b/networking_odl/tests/unit/db/test_db.py
@@ -19,13 +19,13 @@ import operator
 
 import mock
 
+from sqlalchemy.orm import exc
+
 from networking_odl.common import constants as odl_const
 from networking_odl.db import db
 from networking_odl.db import models
 from networking_odl.tests.unit import test_base_db
 
-from oslo_db.exception import DBDeadlock
-
 
 class DbTestCase(test_base_db.ODLBaseDbTestCase):
 
@@ -144,9 +144,10 @@ class DbTestCase(test_base_db.ODLBaseDbTestCase):
         row = db.get_oldest_pending_db_row_with_lock(self.db_session)
         self.assertEqual(older_row, row)
 
-    def test_get_oldest_pending_row_when_deadlock(self):
+    def test_get_oldest_pending_row_when_conflict(self):
         db.create_pending_row(self.db_session, *self.UPDATE_ROW)
-        update_mock = mock.MagicMock(side_effect=(DBDeadlock, mock.DEFAULT))
+        update_mock = mock.MagicMock(
+            side_effect=(exc.StaleDataError, mock.DEFAULT))
 
         # Mocking is mandatory to achieve a deadlock regardless of the DB
         # backend being used when running the tests