removed db_exc.DBDuplicateEntry in bw_usage_update

BandwidthUsage model has no UniqueConstraints.
In 'bw_usage_cache' table in nova db there is single primary
autoincrement key. So the duplicate entry problem is solved by
db itself and db_exc.DBDuplicateEntry could not be raised in Nova.

Ideally we should add UniqueConstraint to prevent multiple bw usage
records existing for the same date range and UUID. That fix for this
will mean we should be able to remove the .first() call and instead
use .one(). The current code that uses .first() is not correct
because there is no order_by() applied on the SQL query and
therefore the returned "first record" is indeterminate.

This workaround fix removed misleading note and exception and
added order_by() to ensure that the same record is updated every time.

Co-Authored-By: Sergey Nikitin <snikitin@mirantis.com>

Closes-bug: #1614561

Change-Id: I408bc3a3e5623965a619d8c7241e4e77c8bf44f5

nova/db/sqlalchemy/api.py

@@ -5482,11 +5482,15 @@ def bw_usage_update(context, uuid, mac, start_period, bw_in, bw_out,
               'last_ctr_out': last_ctr_out,
               'bw_in': bw_in,
               'bw_out': bw_out}
+    # NOTE(pkholkin): order_by() is needed here to ensure that the
+    # same record is updated every time. It can be removed after adding
+    # unique constraint to this model.
     bw_usage = model_query(context, models.BandwidthUsage,
             read_deleted='yes').\
                     filter_by(start_period=ts_values['start_period']).\
                     filter_by(uuid=uuid).\
-                    filter_by(mac=mac).first()
+                    filter_by(mac=mac).\
+                    order_by(asc(models.BandwidthUsage.id)).first()
 
     if bw_usage:
         bw_usage.update(values)
@@ -5501,12 +5505,8 @@ def bw_usage_update(context, uuid, mac, start_period, bw_in, bw_out,
     bwusage.bw_out = bw_out
     bwusage.last_ctr_in = last_ctr_in
     bwusage.last_ctr_out = last_ctr_out
-    try:
-        bwusage.save(context.session)
-    except db_exc.DBDuplicateEntry:
-        # NOTE(sirp): Possible race if two greenthreads attempt to create
-        # the usage entry at the same time. First one wins.
-        pass
+    bwusage.save(context.session)
+
     return bwusage
 
 

nova/tests/unit/db/test_db_api.py

@@ -8611,6 +8611,60 @@ class BwUsageTestCase(test.TestCase, ModelsObjectComparatorMixin):
         self._assertEqualObjects(expected_bw_usage, bw_usage,
                                  ignored_keys=self._ignored_keys)
 
+    def _create_bw_usage(self, context, uuid, mac, start_period, bw_in, bw_out,
+                         last_ctr_in, last_ctr_out, id, last_refreshed=None):
+        with sqlalchemy_api.get_context_manager(context).writer.using(context):
+            bwusage = models.BandwidthUsage()
+            bwusage.start_period = start_period
+            bwusage.uuid = uuid
+            bwusage.mac = mac
+            bwusage.last_refreshed = last_refreshed
+            bwusage.bw_in = bw_in
+            bwusage.bw_out = bw_out
+            bwusage.last_ctr_in = last_ctr_in
+            bwusage.last_ctr_out = last_ctr_out
+            bwusage.id = id
+            bwusage.save(context.session)
+
+    def test_bw_usage_update_exactly_one_record(self):
+        now = timeutils.utcnow()
+        start_period = now - datetime.timedelta(seconds=10)
+        uuid = 'fake_uuid'
+
+        # create two equal bw_usages with IDs 1 and 2
+        for id in range(1, 3):
+            bw_usage = {'uuid': uuid,
+                        'mac': 'fake_mac',
+                        'start_period': start_period,
+                        'bw_in': 100,
+                        'bw_out': 200,
+                        'last_ctr_in': 12345,
+                        'last_ctr_out': 67890,
+                        'last_refreshed': now,
+                        'id': id}
+            self._create_bw_usage(self.ctxt, **bw_usage)
+
+        # check that we have two equal bw_usages
+        self.assertEqual(
+            2, len(db.bw_usage_get_by_uuids(self.ctxt, [uuid], start_period)))
+
+        # update 'last_ctr_in' field in one bw_usage
+        updated_bw_usage = {'uuid': uuid,
+                            'mac': 'fake_mac',
+                            'start_period': start_period,
+                            'bw_in': 100,
+                            'bw_out': 200,
+                            'last_ctr_in': 54321,
+                            'last_ctr_out': 67890,
+                            'last_refreshed': now}
+        result = db.bw_usage_update(
+            self.ctxt, update_cells=False, **updated_bw_usage)
+
+        # check that only bw_usage with ID 1 was updated
+        self.assertEqual(1, result['id'])
+        self._assertEqualObjects(updated_bw_usage, result,
+                                 ignored_keys=self._ignored_keys)
+
     def test_bw_usage_get(self):
         now = timeutils.utcnow()
         start_period = now - datetime.timedelta(seconds=10)