Skip to content

Commit

Permalink
Avoid lazy-loading projects during flavor notification
Browse files Browse the repository at this point in the history
This makes the flavor notification not load the flavor's projects
if they are not already set. Even though we attempted to orphan the
instance-bound flavors in a recent change[1], there are other times
where we were loading projects purely for the notification. This
lazy-load physically will not work if we're in the cell, and is
never something we want to do when we're an instance-bound flavor.

This makes us check to see if projects are loaded, and if so, clone
the flavor to notify on, and set projects on that clone to an
empty list.

[1]: This effectively reverts I458f81931bad7874e951e1c0fd464d149f61b244
which solved the problem in some places, but not all.

Change-Id: I0f80f101a528ab9cda81d025cf316f2ac16bdcb7
  • Loading branch information
kk7ds committed Mar 16, 2017
1 parent 03d0406 commit 5a363a0
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 37 deletions.
7 changes: 7 additions & 0 deletions nova/notifications/objects/flavor.py
Expand Up @@ -75,6 +75,13 @@ class FlavorPayload(base.NotificationPayloadBase):

def __init__(self, flavor, **kwargs):
super(FlavorPayload, self).__init__(**kwargs)
if 'projects' not in flavor:
# NOTE(danms): If projects is not loaded in the flavor,
# don't attempt to load it. If we're in a child cell then
# we can't load the real flavor, and if we're a flavor on
# an instance then we don't want to anyway.
flavor = flavor.obj_clone()
flavor._context = None
self.populate_schema(flavor=flavor)

def obj_make_compatible(self, primitive, target_version):
Expand Down
5 changes: 5 additions & 0 deletions nova/objects/flavor.py
Expand Up @@ -610,6 +610,11 @@ def destroy(self):
db.flavor_destroy(self._context, self.flavorid)

def _send_notification(self, action):
# NOTE(danms): Instead of making the below notification
# lazy-load projects (which is a problem for instance-bound
# flavors and compute-cell operations), just load them here.
if 'projects' not in self:
self._load_projects()
notification_type = flavor_notification.FlavorNotification
payload_type = flavor_notification.FlavorPayload

Expand Down
13 changes: 0 additions & 13 deletions nova/objects/instance.py
Expand Up @@ -272,19 +272,6 @@ def _obj_from_primitive(cls, context, objver, primitive):
self = super(Instance, cls)._obj_from_primitive(context, objver,
primitive)
self._reset_metadata_tracking()

# Note(gibi): The flavors stored in the instance should not be used
# for lazy load data from the db as these objects aren't connected to
# real flavor objects in the db. However during RPC deserialization
# every object gets a valid context so we have to orphan the flavors
# again.
if self.obj_attr_is_set('flavor'):
self.flavor._context = None
if self.obj_attr_is_set('old_flavor') and self.old_flavor:
self.old_flavor._context = None
if self.obj_attr_is_set('new_flavor') and self.new_flavor:
self.new_flavor._context = None

return self

@property
Expand Down
24 changes: 0 additions & 24 deletions nova/tests/unit/objects/test_instance.py
Expand Up @@ -1518,30 +1518,6 @@ def test_obj_clone(self):
inst1 = inst1.obj_clone()
self.assertEqual(len(inst1.obj_what_changed()), 0)

def test_flavor_deserialization_orphans_flavor(self):
flavor = objects.Flavor(context=self.context,
name='test-flavor',
memory_mb=1024,
root_gb=0,
vcpus=1,
flavorid="m1.test")
flavor.create()
inst = objects.Instance(context=self.context,
user_id=self.context.user_id,
project_id=self.context.project_id,
flavor=flavor,
old_flavor=flavor,
new_flavor=flavor)
inst.create()

inst = objects.Instance.get_by_uuid(self.context,
uuid=inst.uuid,
expected_attrs=['flavor'])

self.assertIsNone(inst.flavor._context)
self.assertIsNone(inst.old_flavor._context)
self.assertIsNone(inst.new_flavor._context)


class TestInstanceObject(test_objects._LocalTest,
_TestInstanceObject):
Expand Down

0 comments on commit 5a363a0

Please sign in to comment.