Change lazy='dynamic' to 'joined' in models_v2

Previously several models were defined with lazy='dynamic'. This would
then cause Ports.fixed_ips (or w/e was specified as dynamic) to be defined
as a AppenderQuery object and when accessed would query for the result.
Therefore in get_ports() for example, when fixed_ips was accessed
in _make_port_dict() we would be issuing an addition query to the database
to lookup the fixed_ips information.

This patch changes update_port/subnet() so that it keeps track of what
changed so it does not have to query the db for values it should already
know about.

This patch also adds a unit test for update_subnet() adding host routes
where there was missing code coverage.

Implements blueprint improve-db-performance

Change-Id: I0b2214604042a1fd362cbbf3fd70e31adf0ce279

diff --git a/quantum/db/db_base_plugin_v2.py b/quantum/db/db_base_plugin_v2.py
index 39cd5310b4776e84e67b9e63cebf63000d31bf33..225ad703a52491106b20e8068732834abf65acc3 100644 (file)
--- a/quantum/db/db_base_plugin_v2.py
+++ b/quantum/db/db_base_plugin_v2.py
@@ -671,6 +671,8 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2,
                              new_ips):
         """Add or remove IPs from the port."""
         ips = []
+        # These ips are still on the port and haven't been removed
+        prev_ips = []
 
         # the new_ips contain all of the fixed_ips that are to be updated
         if len(new_ips) > cfg.CONF.max_fixed_ips_per_port:
@@ -684,6 +686,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2,
                     original_ip['ip_address'] == new_ip['ip_address']):
                     original_ips.remove(original_ip)
                     new_ips.remove(new_ip)
+                    prev_ips.append(original_ip)
 
         # Check if the IP's to add are OK
         to_add = self._test_fixed_ips_for_port(context, network_id, new_ips)
@@ -699,7 +702,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2,
             LOG.debug(_("Port update. Adding %s"), to_add)
             network = self._get_network(context, network_id)
             ips = self._allocate_fixed_ips(context, network, to_add)
-        return ips
+        return ips, prev_ips
 
     def _allocate_ips_for_port(self, context, network, port):
         """Allocate IP addresses for the port.
@@ -1186,6 +1189,8 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2,
         dns lease or we support gratuitous DHCP offers
         """
         s = subnet['subnet']
+        changed_host_routes = False
+        changed_dns = False
         db_subnet = self._get_subnet(context, id)
         # Fill 'ip_version' and 'allocation_pools' fields with the current
         # value since _validate_subnet() expects subnet spec has 'ip_version'
@@ -1201,11 +1206,13 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2,
 
         with context.session.begin(subtransactions=True):
             if "dns_nameservers" in s:
+                changed_dns = True
                 old_dns_list = self._get_dns_by_subnet(context, id)
                 new_dns_addr_set = set(s["dns_nameservers"])
                 old_dns_addr_set = set([dns['address']
                                         for dns in old_dns_list])
 
+                new_dns = list(new_dns_addr_set)
                 for dns_addr in old_dns_addr_set - new_dns_addr_set:
                     for dns in old_dns_list:
                         if dns['address'] == dns_addr:
@@ -1221,6 +1228,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2,
                 return ht['destination'] + "_" + ht['nexthop']
 
             if "host_routes" in s:
+                changed_host_routes = True
                 old_route_list = self._get_route_by_subnet(context, id)
 
                 new_route_set = set([_combine(route)
@@ -1239,11 +1247,24 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2,
                         nexthop=route_str.partition("_")[2],
                         subnet_id=id)
                     context.session.add(route)
+
+                # Gather host routes for result
+                new_routes = []
+                for route_str in new_route_set:
+                    new_routes.append(
+                        {'destination': route_str.partition("_")[0],
+                         'nexthop': route_str.partition("_")[2]})
                 del s["host_routes"]
 
             subnet = self._get_subnet(context, id)
             subnet.update(s)
-        return self._make_subnet_dict(subnet)
+        result = self._make_subnet_dict(subnet)
+        # Keep up with fields that changed
+        if changed_dns:
+            result['dns_nameservers'] = new_dns
+        if changed_host_routes:
+            result['host_routes'] = new_routes
+        return result
 
     def delete_subnet(self, context, id):
         with context.session.begin(subtransactions=True):
@@ -1357,20 +1378,21 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2,
     def update_port(self, context, id, port):
         p = port['port']
 
+        changed_ips = False
         with context.session.begin(subtransactions=True):
             port = self._get_port(context, id)
             # Check if the IPs need to be updated
             if 'fixed_ips' in p:
+                changed_ips = True
                 self._recycle_expired_ip_allocations(context,
                                                      port['network_id'])
                 original = self._make_port_dict(port, process_extensions=False)
-                ips = self._update_ips_for_port(context,
-                                                port["network_id"],
-                                                id,
-                                                original["fixed_ips"],
-                                                p['fixed_ips'])
+                added_ips, prev_ips = self._update_ips_for_port(
+                    context, port["network_id"], id, original["fixed_ips"],
+                    p['fixed_ips'])
+
                 # Update ips if necessary
-                for ip in ips:
+                for ip in added_ips:
                     allocated = models_v2.IPAllocation(
                         network_id=port['network_id'], port_id=port.id,
                         ip_address=ip['ip_address'], subnet_id=ip['subnet_id'],
@@ -1380,7 +1402,11 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2,
             # and then update the port
             port.update(self._filter_non_model_columns(p, models_v2.Port))
 
-        return self._make_port_dict(port)
+        result = self._make_port_dict(port)
+        # Keep up with fields that changed
+        if changed_ips:
+            result['fixed_ips'] = prev_ips + added_ips
+        return result
 
     def delete_port(self, context, id):
         with context.session.begin(subtransactions=True):

diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py
index ea7625a67b7e81f3638b91cd1fde2406f0547746..22fcf07f3861cb56bf1891cff46b82c98662badd 100644 (file)
--- a/quantum/db/l3_db.py
+++ b/quantum/db/l3_db.py
@@ -749,7 +749,7 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                                        DEVICE_OWNER_FLOATINGIP]:
             # Raise port in use only if the port has IP addresses
             # Otherwise it's a stale port that can be removed
-            fixed_ips = port_db['fixed_ips'].all()
+            fixed_ips = port_db['fixed_ips']
             if fixed_ips:
                 raise l3.L3PortInUse(port_id=port_id,
                                      device_owner=port_db['device_owner'])

diff --git a/quantum/db/models_v2.py b/quantum/db/models_v2.py
index 1840fb654ee17533be92969dcdd1f4bba98bbe1d..f7e1aa1f46aae1321a0760aa0286c4c88d85e864 100644 (file)
--- a/quantum/db/models_v2.py
+++ b/quantum/db/models_v2.py
@@ -71,7 +71,7 @@ class IPAllocationPool(model_base.BASEV2, HasId):
     last_ip = sa.Column(sa.String(64), nullable=False)
     available_ranges = orm.relationship(IPAvailabilityRange,
                                         backref='ipallocationpool',
-                                        lazy="dynamic",
+                                        lazy="joined",
                                         cascade='delete')
 
     def __repr__(self):
@@ -116,7 +116,7 @@ class Port(model_base.BASEV2, HasId, HasTenant):
     name = sa.Column(sa.String(255))
     network_id = sa.Column(sa.String(36), sa.ForeignKey("networks.id"),
                            nullable=False)
-    fixed_ips = orm.relationship(IPAllocation, backref='ports', lazy="dynamic")
+    fixed_ips = orm.relationship(IPAllocation, backref='ports', lazy='joined')
     mac_address = sa.Column(sa.String(32), nullable=False)
     admin_state_up = sa.Column(sa.Boolean(), nullable=False)
     status = sa.Column(sa.String(16), nullable=False)
@@ -148,7 +148,7 @@ class Subnet(model_base.BASEV2, HasId, HasTenant):
     gateway_ip = sa.Column(sa.String(64))
     allocation_pools = orm.relationship(IPAllocationPool,
                                         backref='subnet',
-                                        lazy="dynamic",
+                                        lazy="joined",
                                         cascade='delete')
     enable_dhcp = sa.Column(sa.Boolean())
     dns_nameservers = orm.relationship(DNSNameServer,

diff --git a/quantum/tests/unit/test_db_plugin.py b/quantum/tests/unit/test_db_plugin.py
index d2280a03eadb23dd514faadedb34886355a328d4..b77835e161a8859a173c1602f749c95f786702ec 100644 (file)
--- a/quantum/tests/unit/test_db_plugin.py
+++ b/quantum/tests/unit/test_db_plugin.py
@@ -2976,6 +2976,35 @@ class TestSubnetsV2(QuantumDbPluginV2TestCase):
             self.assertEqual(res['subnet']['gateway_ip'],
                              data['subnet']['gateway_ip'])
 
+    def test_update_subnet_adding_additional_host_routes_and_dns(self):
+        host_routes = [{'destination': '172.16.0.0/24',
+                        'nexthop': '10.0.2.2'}]
+        with self.network() as network:
+            data = {'subnet': {'network_id': network['network']['id'],
+                               'cidr': '10.0.2.0/24',
+                               'ip_version': 4,
+                               'dns_nameservers': ['192.168.0.1'],
+                               'host_routes': host_routes,
+                               'tenant_id': network['network']['tenant_id']}}
+            subnet_req = self.new_create_request('subnets', data)
+            res = self.deserialize(self.fmt, subnet_req.get_response(self.api))
+
+            host_routes = [{'destination': '172.16.0.0/24',
+                            'nexthop': '10.0.2.2'},
+                           {'destination': '192.168.0.0/24',
+                            'nexthop': '10.0.2.3'}]
+
+            dns_nameservers = ['192.168.0.1', '192.168.0.2']
+            data = {'subnet': {'host_routes': host_routes,
+                               'dns_nameservers': dns_nameservers}}
+            req = self.new_update_request('subnets', data,
+                                          res['subnet']['id'])
+            res = self.deserialize(self.fmt, req.get_response(self.api))
+            self.assertEquals(sorted(res['subnet']['host_routes']),
+                              sorted(host_routes))
+            self.assertEquals(sorted(res['subnet']['dns_nameservers']),
+                              sorted(dns_nameservers))
+
     def test_update_subnet_shared_returns_400(self):
         with self.network(shared=True) as network:
             with self.subnet(network=network) as subnet: