Don't persist could-be-stale InstanceGroup fields in RequestSpec
Not only might these be stale, but they can also be very large, especially if InstanceGroup.members has a lot of peer instance uuids. This can overrun a 64kb TEXT field and cause a failure. Related-Bug: #1738094 Change-Id: Ie70c77db753711e1449e99534d3b83669871943f
This commit is contained in:
parent
29e453df8e
commit
94fd36f058
|
@ -445,6 +445,18 @@ class RequestSpec(base.NovaObject):
|
|||
else:
|
||||
setattr(spec, key, getattr(spec_obj, key))
|
||||
spec._context = context
|
||||
|
||||
if 'instance_group' in spec and spec.instance_group:
|
||||
# NOTE(danms): We don't store the full instance group in
|
||||
# the reqspec since it would be stale almost immediately.
|
||||
# Instead, load it by uuid here so it's up-to-date.
|
||||
try:
|
||||
spec.instance_group = objects.InstanceGroup.get_by_uuid(
|
||||
context, spec.instance_group.uuid)
|
||||
except exception.InstanceGroupNotFound:
|
||||
# NOTE(danms): Instance group may have been deleted
|
||||
spec.instance_group = None
|
||||
|
||||
spec.obj_reset_changes()
|
||||
return spec
|
||||
|
||||
|
@ -484,7 +496,14 @@ class RequestSpec(base.NovaObject):
|
|||
# NOTE(alaski): The db schema is the full serialized object in a
|
||||
# 'spec' column. If anything has changed we rewrite the full thing.
|
||||
if updates:
|
||||
db_updates = {'spec': jsonutils.dumps(self.obj_to_primitive())}
|
||||
# NOTE(danms): Don't persist the could-be-large and could-be-stale
|
||||
# properties of InstanceGroup
|
||||
spec = self.obj_clone()
|
||||
if 'instance_group' in spec and spec.instance_group:
|
||||
spec.instance_group.members = None
|
||||
spec.instance_group.hosts = None
|
||||
|
||||
db_updates = {'spec': jsonutils.dumps(spec.obj_to_primitive())}
|
||||
if 'instance_uuid' in updates:
|
||||
db_updates['instance_uuid'] = updates['instance_uuid']
|
||||
return db_updates
|
||||
|
|
|
@ -30,6 +30,9 @@ class RequestSpecTestCase(test.NoDBTestCase):
|
|||
def setUp(self):
|
||||
super(RequestSpecTestCase, self).setUp()
|
||||
self.useFixture(fixtures.Database(database='api'))
|
||||
# NOTE(danms): Only needed for the fallback legacy main db loading
|
||||
# code in InstanceGroup.
|
||||
self.useFixture(fixtures.Database(database='main'))
|
||||
self.context = context.RequestContext('fake-user', 'fake-project')
|
||||
self.spec_obj = request_spec.RequestSpec()
|
||||
self.instance_uuid = None
|
||||
|
|
|
@ -16,6 +16,7 @@ from oslo_utils import uuidutils
|
|||
from nova import context
|
||||
from nova import objects
|
||||
from nova.tests.unit import fake_flavor
|
||||
from nova.tests import uuidsentinel as uuids
|
||||
|
||||
|
||||
INSTANCE_NUMA_TOPOLOGY = objects.InstanceNUMATopology(
|
||||
|
@ -77,7 +78,7 @@ def fake_spec_obj(remove_id=False):
|
|||
req_obj.flavor = fake_flavor.fake_flavor_obj(ctxt)
|
||||
req_obj.retry = objects.SchedulerRetries()
|
||||
req_obj.limits = objects.SchedulerLimits()
|
||||
req_obj.instance_group = objects.InstanceGroup()
|
||||
req_obj.instance_group = objects.InstanceGroup(uuid=uuids.instgroup)
|
||||
req_obj.project_id = 'fake'
|
||||
req_obj.num_instances = 1
|
||||
req_obj.availability_zone = None
|
||||
|
|
|
@ -494,9 +494,11 @@ class _TestRequestSpecObject(object):
|
|||
|
||||
@mock.patch.object(request_spec.RequestSpec,
|
||||
'_get_by_instance_uuid_from_db')
|
||||
def test_get_by_instance_uuid(self, get_by_uuid):
|
||||
@mock.patch('nova.objects.InstanceGroup.get_by_uuid')
|
||||
def test_get_by_instance_uuid(self, mock_get_ig, get_by_uuid):
|
||||
fake_spec = fake_request_spec.fake_db_spec()
|
||||
get_by_uuid.return_value = fake_spec
|
||||
mock_get_ig.return_value = objects.InstanceGroup(name='fresh')
|
||||
|
||||
req_obj = request_spec.RequestSpec.get_by_instance_uuid(self.context,
|
||||
fake_spec['instance_uuid'])
|
||||
|
@ -517,6 +519,7 @@ class _TestRequestSpecObject(object):
|
|||
self.assertIsInstance(req_obj.retry, objects.SchedulerRetries)
|
||||
self.assertIsInstance(req_obj.limits, objects.SchedulerLimits)
|
||||
self.assertIsInstance(req_obj.instance_group, objects.InstanceGroup)
|
||||
self.assertEqual('fresh', req_obj.instance_group.name)
|
||||
|
||||
def _check_update_primitive(self, req_obj, changes):
|
||||
self.assertEqual(req_obj.instance_uuid, changes['instance_uuid'])
|
||||
|
@ -532,11 +535,14 @@ class _TestRequestSpecObject(object):
|
|||
|
||||
# object fields
|
||||
for field in ['image', 'numa_topology', 'pci_requests', 'flavor',
|
||||
'retry', 'limits', 'instance_group']:
|
||||
'retry', 'limits']:
|
||||
self.assertEqual(
|
||||
getattr(req_obj, field).obj_to_primitive(),
|
||||
getattr(serialized_obj, field).obj_to_primitive())
|
||||
|
||||
self.assertIsNone(serialized_obj.instance_group.members)
|
||||
self.assertIsNone(serialized_obj.instance_group.hosts)
|
||||
|
||||
def test_create(self):
|
||||
req_obj = fake_request_spec.fake_spec_obj(remove_id=True)
|
||||
|
||||
|
|
Loading…
Reference in New Issue