objects: don't refetch a non-list object field if it's None
When an object field has the value of corresponding model attribute set to None, it means that it's indeed unset, and there is no need to refetch it from the database (it will still be None). This becomes a more noticeable problem when we introduce security field to Network (part of I57395d0f646ffa3089c1ac6c5a68d952ccd0b42c) that is exactly this type of field, because with the auto-expiry patch included (I0d65d19204da8ce30addfa5faff68544534b7853) those redundant fetches trigger actual SELECT statements that affect performance and break test_get_objects_queries_constant regression test case. The patch also changes the type of 'distributed_binding' Port object field from ObjectField to ListOfObjectFields, and also renames the field into 'distributed_bindings' to reflect actual possibilities allowed by database schema. Note this patch is NOT expected to enable actual support for multiple binding values, which would belong to a separate patch, but just attempts to apply the minimal needed changes required because of the other changes included while making sure that consequent changes for the field don't need to change object definition. Partially-Implements: blueprint adopt-oslo-versioned-objects-for-db Change-Id: I833b07fdbb4a57bdf8bd4255e61098ec512d1a5b
This commit is contained in:
Ihar Hrachyshka
Vu Cong Tuan
parent
a388701ddf
commit
752c33e92b
|
|
@ -713,8 +713,9 @@ class NeutronDbObject(NeutronObject):
|
|||
# subclasses=True
|
||||
for field in self.synthetic_fields:
|
||||
try:
|
||||
field_def = self.fields[field]
|
||||
objclasses = NeutronObjectRegistry.obj_classes(
|
||||
).get(self.fields[field].objname)
|
||||
).get(field_def.objname)
|
||||
except AttributeError:
|
||||
# NOTE(rossella_s) this is probably because this field is not
|
||||
# an ObjectField
|
||||
|
|
@ -735,25 +736,28 @@ class NeutronDbObject(NeutronObject):
|
|||
|
||||
synthetic_field_db_name = (
|
||||
self.fields_need_translation.get(field, field))
|
||||
synth_db_objs = (db_obj.get(synthetic_field_db_name, None)
|
||||
if db_obj else None)
|
||||
|
||||
# synth_db_objs can be list, empty list or None, that is why
|
||||
# we need 'is not None', because [] is valid case for 'True'
|
||||
if synth_db_objs is not None:
|
||||
if not isinstance(synth_db_objs, list):
|
||||
synth_db_objs = [synth_db_objs]
|
||||
synth_objs = [objclass._load_object(self.obj_context, obj)
|
||||
for obj in synth_db_objs]
|
||||
else:
|
||||
synth_objs = objclass.get_objects(
|
||||
self.obj_context, **{
|
||||
k: getattr(self, v) if v in self else db_obj.get(v)
|
||||
for k, v in foreign_keys.items()})
|
||||
if isinstance(self.fields[field], obj_fields.ObjectField):
|
||||
setattr(self, field, synth_objs[0] if synth_objs else None)
|
||||
else:
|
||||
if isinstance(field_def, obj_fields.ListOfObjectsField):
|
||||
synth_db_objs = (db_obj.get(synthetic_field_db_name, None)
|
||||
if db_obj else None)
|
||||
if synth_db_objs is not None:
|
||||
synth_objs = [objclass._load_object(self.obj_context, obj)
|
||||
for obj in synth_db_objs]
|
||||
else:
|
||||
synth_objs = objclass.get_objects(
|
||||
self.obj_context, **{
|
||||
k: getattr(self, v) if v in self else db_obj.get(v)
|
||||
for k, v in foreign_keys.items()})
|
||||
setattr(self, field, synth_objs)
|
||||
else:
|
||||
synth_db_obj = (db_obj.get(synthetic_field_db_name, None)
|
||||
if db_obj else None)
|
||||
if synth_db_obj:
|
||||
synth_db_obj = objclass._load_object(self.obj_context,
|
||||
synth_db_obj)
|
||||
setattr(self, field, synth_db_obj)
|
||||
self.obj_reset_changes([field])
|
||||
|
||||
def create(self):
|
||||
|
|
|
|||
|
|
@ -263,7 +263,8 @@ class Port(base.NeutronDbObject):
|
|||
# Version 1.0: Initial version
|
||||
# Version 1.1: Add data_plane_status field
|
||||
# Version 1.2: Added segment_id to binding_levels
|
||||
VERSION = '1.2'
|
||||
# Version 1.3: distributed_binding -> distributed_bindings
|
||||
VERSION = '1.3'
|
||||
|
||||
db_model = models_v2.Port
|
||||
|
||||
|
|
@ -290,7 +291,7 @@ class Port(base.NeutronDbObject):
|
|||
'dhcp_options': obj_fields.ListOfObjectsField(
|
||||
'ExtraDhcpOpt', nullable=True
|
||||
),
|
||||
'distributed_binding': obj_fields.ObjectField(
|
||||
'distributed_bindings': obj_fields.ListOfObjectsField(
|
||||
'DistributedPortBinding', nullable=True
|
||||
),
|
||||
'dns': obj_fields.ObjectField('PortDNS', nullable=True),
|
||||
|
|
@ -326,7 +327,7 @@ class Port(base.NeutronDbObject):
|
|||
'binding_levels',
|
||||
'data_plane_status',
|
||||
'dhcp_options',
|
||||
'distributed_binding',
|
||||
'distributed_bindings',
|
||||
'dns',
|
||||
'fixed_ips',
|
||||
'qos_policy_id',
|
||||
|
|
@ -337,7 +338,7 @@ class Port(base.NeutronDbObject):
|
|||
fields_need_translation = {
|
||||
'binding': 'port_binding',
|
||||
'dhcp_options': 'dhcp_opts',
|
||||
'distributed_binding': 'distributed_port_binding',
|
||||
'distributed_bindings': 'distributed_port_binding',
|
||||
'security': 'port_security',
|
||||
}
|
||||
|
||||
|
|
@ -430,11 +431,12 @@ class Port(base.NeutronDbObject):
|
|||
if 'mac_address' in fields:
|
||||
fields['mac_address'] = utils.AuthenticEUI(fields['mac_address'])
|
||||
|
||||
distributed_port_binding = fields.get('distributed_binding')
|
||||
distributed_port_binding = fields.get('distributed_bindings')
|
||||
if distributed_port_binding:
|
||||
fields['distributed_binding'] = fields['distributed_binding'][0]
|
||||
# TODO(ihrachys) support multiple bindings
|
||||
fields['distributed_bindings'] = fields['distributed_bindings'][0]
|
||||
else:
|
||||
fields['distributed_binding'] = None
|
||||
fields['distributed_bindings'] = []
|
||||
return fields
|
||||
|
||||
def from_db_object(self, db_obj):
|
||||
|
|
@ -467,6 +469,10 @@ class Port(base.NeutronDbObject):
|
|||
for lvl in binding_levels:
|
||||
lvl['versioned_object.version'] = '1.0'
|
||||
lvl['versioned_object.data'].pop('segment_id', None)
|
||||
if _target_version < (1, 3):
|
||||
bindings = primitive.pop('distributed_bindings', [])
|
||||
primitive['distributed_binding'] = (bindings[0]
|
||||
if bindings else None)
|
||||
|
||||
@classmethod
|
||||
def get_ports_by_router(cls, context, router_id, owner, subnet):
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ object_data = {
|
|||
'NetworkPortSecurity': '1.0-b30802391a87945ee9c07582b4ff95e3',
|
||||
'NetworkRBAC': '1.0-c8a67f39809c5a3c8c7f26f2f2c620b2',
|
||||
'NetworkSegment': '1.0-57b7f2960971e3b95ded20cbc59244a8',
|
||||
'Port': '1.2-5bf48d12a7bf7f5b7a319e8003b437a5',
|
||||
'Port': '1.3-4cb798ffc8b08f2657c0bd8afa708e9e',
|
||||
'PortBinding': '1.0-3306deeaa6deb01e33af06777d48d578',
|
||||
'PortBindingLevel': '1.1-50d47f63218f87581b6cd9a62db574e5',
|
||||
'PortDataPlaneStatus': '1.0-25be74bda46c749653a10357676c0ab2',
|
||||
|
|
|
|||
|
|
@ -393,3 +393,28 @@ class PortDbObjectTestCase(obj_test_base.BaseDbObjectTestCase,
|
|||
# downgrade of the binding object
|
||||
binding_v1_0 = binding.obj_to_primitive(target_version='1.0')
|
||||
self.assertEqual(binding_v1_0, lvl)
|
||||
|
||||
def test_v1_3_to_v1_2_unlists_distributed_bindings(self):
|
||||
port_new = self._create_test_port()
|
||||
|
||||
# empty list transforms into None
|
||||
port_v1_2 = port_new.obj_to_primitive(target_version='1.2')
|
||||
port_data = port_v1_2['versioned_object.data']
|
||||
self.assertIsNone(port_data['distributed_binding'])
|
||||
|
||||
# now insert a distributed binding
|
||||
binding = ports.DistributedPortBinding(
|
||||
self.context,
|
||||
host='host1', port_id=port_new.id, status='ACTIVE',
|
||||
vnic_type='vnic_type1', vif_type='vif_type1')
|
||||
binding.create()
|
||||
|
||||
# refetch port object to include binding
|
||||
port_new = ports.Port.get_object(self.context, id=port_new.id)
|
||||
|
||||
# new primitive should contain the binding data
|
||||
port_v1_2 = port_new.obj_to_primitive(target_version='1.2')
|
||||
port_data = port_v1_2['versioned_object.data']
|
||||
binding_data = (
|
||||
port_data['distributed_binding']['versioned_object.data'])
|
||||
self.assertEqual(binding.host, binding_data['host'])
|
||||
|
|
|
|||
Loading…
Reference in New Issue