Merge "Improve bw_usage_update() performance"

4 files changed, 92 insertions, 30 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 14e84fa1b..5773afc41 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -36,7 +36,6 @@ terminating it.
 
 import contextlib
 import functools
-import inspect
 import socket
 import sys
 import time
@@ -2598,12 +2597,16 @@ class ComputeManager(manager.SchedulerDependentManager):
                 # they just don't get the info in the usage events.
                 return
 
+            refreshed = timeutils.utcnow()
             for usage in bw_usage:
+                # Allow switching of greenthreads between queries.
+                greenthread.sleep(0)
                 self.db.bw_usage_update(context,
                                         usage['uuid'],
                                         usage['mac_address'],
                                         start_time,
-                                        usage['bw_in'], usage['bw_out'])
+                                        usage['bw_in'], usage['bw_out'],
+                                        last_refreshed=refreshed)
 
     @manager.periodic_task
     def _report_driver_status(self, context):
diff --git a/nova/db/api.py b/nova/db/api.py
index f0d6c435d..cbbc105c7 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -1594,18 +1594,13 @@ def bw_usage_get_by_uuids(context, uuids, start_period):
     return IMPL.bw_usage_get_by_uuids(context, uuids, start_period)
 
 
-def bw_usage_update(context,
-                    uuid,
-                    mac,
-                    start_period,
-                    bw_in, bw_out):
-    """Update cached bw usage for an instance and network
-       Creates new record if needed."""
-    return IMPL.bw_usage_update(context,
-                                uuid,
-                                mac,
-                                start_period,
-                                bw_in, bw_out)
+def bw_usage_update(context, uuid, mac, start_period, bw_in, bw_out,
+                    last_refreshed=None):
+    """Update cached bandwidth usage for an instance's network based on mac
+    address.  Creates new record if needed.
+    """
+    return IMPL.bw_usage_update(context, uuid, mac, start_period, bw_in,
+            bw_out, last_refreshed=last_refreshed)
 
 
 ####################
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index c54388f07..f5697a771 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -4366,30 +4366,35 @@ def bw_usage_get_by_uuids(context, uuids, start_period):
 
 
 @require_context
-def bw_usage_update(context,
-                    uuid,
-                    mac,
-                    start_period,
-                    bw_in, bw_out,
-                    session=None):
+def bw_usage_update(context, uuid, mac, start_period, bw_in, bw_out,
+                    last_refreshed=None, session=None):
     if not session:
         session = get_session()
 
+    if last_refreshed is None:
+        last_refreshed = timeutils.utcnow()
+
+    # NOTE(comstud): More often than not, we'll be updating records vs
+    # creating records.  Optimize accordingly, trying to update existing
+    # records.  Fall back to creation when no rows are updated.
     with session.begin():
-        bwusage = model_query(context, models.BandwidthUsage,
+        values = {'last_refreshed': last_refreshed,
+                  'bw_in': bw_in,
+                  'bw_out': bw_out}
+        rows = model_query(context, models.BandwidthUsage,
                               session=session, read_deleted="yes").\
                       filter_by(start_period=start_period).\
                       filter_by(uuid=uuid).\
                       filter_by(mac=mac).\
-                      first()
-
-        if not bwusage:
-            bwusage = models.BandwidthUsage()
-            bwusage.start_period = start_period
-            bwusage.uuid = uuid
-            bwusage.mac = mac
-
-        bwusage.last_refreshed = timeutils.utcnow()
+                      update(values, synchronize_session=False)
+        if rows:
+            return
+
+        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.save(session=session)
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index 9acc964cf..93e0edfbc 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -459,6 +459,65 @@ class DbApiTestCase(test.TestCase):
         ec2_id = db.get_ec2_snapshot_id_by_uuid(self.context, 'fake-uuid')
         self.assertEqual(ref['id'], ec2_id)
 
+    def test_bw_usage_calls(self):
+        ctxt = context.get_admin_context()
+        now = timeutils.utcnow()
+        timeutils.set_time_override(now)
+        start_period = now - datetime.timedelta(seconds=10)
+        uuid3_refreshed = now - datetime.timedelta(seconds=5)
+
+        expected_bw_usages = [{'uuid': 'fake_uuid1',
+                               'mac': 'fake_mac1',
+                               'start_period': start_period,
+                               'bw_in': 100,
+                               'bw_out': 200,
+                               'last_refreshed': now},
+                              {'uuid': 'fake_uuid2',
+                               'mac': 'fake_mac2',
+                               'start_period': start_period,
+                               'bw_in': 200,
+                               'bw_out': 300,
+                               'last_refreshed': now},
+                              {'uuid': 'fake_uuid3',
+                               'mac': 'fake_mac3',
+                               'start_period': start_period,
+                               'bw_in': 400,
+                               'bw_out': 500,
+                               'last_refreshed': uuid3_refreshed}]
+
+        def _compare(bw_usage, expected):
+            for key, value in expected.items():
+                self.assertEqual(bw_usage[key], value)
+
+        bw_usages = db.bw_usage_get_by_uuids(ctxt,
+                ['fake_uuid1', 'fake_uuid2'], start_period)
+        # No matches
+        self.assertEqual(len(bw_usages), 0)
+
+        # Add 3 entries
+        db.bw_usage_update(ctxt, 'fake_uuid1',
+                'fake_mac1', start_period,
+                100, 200)
+        db.bw_usage_update(ctxt, 'fake_uuid2',
+                'fake_mac2', start_period,
+                100, 200)
+        # Test explicit refreshed time
+        db.bw_usage_update(ctxt, 'fake_uuid3',
+                'fake_mac3', start_period,
+                400, 500, last_refreshed=uuid3_refreshed)
+        # Update 2nd entry
+        db.bw_usage_update(ctxt, 'fake_uuid2',
+                'fake_mac2', start_period,
+                200, 300)
+
+        bw_usages = db.bw_usage_get_by_uuids(ctxt,
+                ['fake_uuid1', 'fake_uuid2', 'fake_uuid3'], start_period)
+        self.assertEqual(len(bw_usages), 3)
+        _compare(bw_usages[0], expected_bw_usages[0])
+        _compare(bw_usages[1], expected_bw_usages[1])
+        _compare(bw_usages[2], expected_bw_usages[2])
+        timeutils.clear_time_override()
+
 
 def _get_fake_aggr_values():
     return {'name': 'fake_aggregate',