openstack/cinder
Code Issues Proposed changes

Merge "Add online data migration routine for attachment_specs"

Browse Source
This commit is contained in:
Zuul 2017-12-22 17:31:29 +00:00 committed by Gerrit Code Review
commit e054357985
5 changed files with 108 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
cinder/cmd/manage.py
View File

@ -250,9 +250,14 @@ class DbCommands(object):
online_migrations = (
# Added in Queens
db.service_uuids_online_data_migration,
# Added in Queens
db.backup_service_online_migration,
# Added in Queens
db.volume_service_uuids_online_data_migration,
# Added in Queens
shared_targets_online_data_migration,
# Added in Queens
db.attachment_specs_online_data_migration
)
def __init__(self):

4
cinder/db/api.py
View File

@ -1820,6 +1820,10 @@ def attachment_specs_update_or_create(context,
attachment_id,
specs)
def attachment_specs_online_data_migration(context, max_count):
return IMPL.attachment_specs_online_data_migration(context, max_count)
###################

24
cinder/db/sqlalchemy/api.py
View File

@ -646,6 +646,30 @@ def volume_service_uuids_online_data_migration(context, max_count):
return total, updated
@enginefacade.writer
def attachment_specs_online_data_migration(context, max_count):
from cinder.objects import volume_attachment
# First figure out how many attachments have specs which need to be
# migrated, grouped by the attachment.id from the specs table.
session = get_session()
total = session.query(models.AttachmentSpecs.attachment_id).filter_by(
deleted=False).group_by(models.AttachmentSpecs.attachment_id).count()
# Now get the limited distinct set of attachment_ids to start migrating.
result = session.query(
models.AttachmentSpecs.attachment_id).filter_by(
deleted=False).group_by(models.AttachmentSpecs.attachment_id).limit(
max_count).all()
migrated = 0
# result is a list of tuples where the first item is the attachment_id
for attachment_id in result:
attachment_id = attachment_id[0]
# Loading the volume attachment object will migrate it's related
# attachment specs and delete those attachment specs.
volume_attachment.VolumeAttachment.get_by_id(context, attachment_id)
migrated += 1
return total, migrated
###################

10
cinder/objects/volume_attachment.py
View File

@ -101,14 +101,14 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject,
attachment.volume = objects.Volume._from_db_object(
context, objects.Volume(), db_volume)
attachment._context = context
attachment.obj_reset_changes()
# This is an online data migration which we should remove when enough
# time has passed and we have a blocker schema migration to check to
# make sure that the attachment_specs table is empty. Operators should
# run the "cinder-manage db online_data_migrations" CLI to force the
# migration on-demand.
# TODO(mriedem): Need a hook for the online_data_migration CLI to query
# the database for all attachment_specs entries and migrate their
# related volume_attachment records using this object.
connector = db.attachment_specs_get(context, attachment.id)
if connector:
# Update ourselves and delete the attachment_specs.
@ -119,8 +119,6 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject,
db.attachment_specs_delete(
context, attachment.id, spec_key)
attachment._context = context
attachment.obj_reset_changes()
return attachment
def obj_load_attr(self, attrname):
@ -182,6 +180,8 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject,
raise exception.ObjectActionError(action='create',
reason=_('already created'))
updates = self.cinder_obj_get_changes()
if 'connector' in updates:
self._convert_connector_to_db_format(updates)
with self.obj_as_admin():
db_attachment = db.volume_attach(self._context, updates)
self._from_db_object(self._context, self, db_attachment)

70
cinder/tests/unit/test_db_api.py
View File

@ -3458,3 +3458,73 @@ class DBAPIGroupTestCase(BaseTest):
self.assertEqual(
new_cluster_name + groups[i].cluster_name[len(cluster_name):],
db_groups[i].cluster_name)
class DBAPIAttachmentSpecsTestCase(BaseTest):
def test_attachment_specs_online_data_migration(self):
"""Tests the online data migration initiated via cinder-manage"""
# Create five attachment records:
# 1. first attachment has specs but is deleted so it's ignored
# 2. second attachment is already migrated (no attachment_specs
# entries) so it's ignored
# 3. the remaining attachments have specs so they are migrated in
# in batches of 2
# Create an attachment record with specs and delete it.
attachment = objects.VolumeAttachment(
self.ctxt, attach_status='attaching', volume_id=fake.VOLUME_ID)
attachment.create()
# Create an attachment_specs entry for attachment.
connector = {'host': '127.0.0.1'}
db.attachment_specs_update_or_create(
self.ctxt, attachment.id, connector)
# Now delete the attachment which should also delete the specs.
attachment.destroy()
# Run the migration routine to see that there is nothing to migrate.
total, migrated = db.attachment_specs_online_data_migration(
self.ctxt, 50)
self.assertEqual(0, total)
self.assertEqual(0, migrated)
# Create a volume attachment with no specs (already migrated).
attachment = objects.VolumeAttachment(
self.ctxt, attach_status='attaching', volume_id=fake.VOLUME_ID,
connector=connector)
attachment.create()
# Run the migration routine to see that there is nothing to migrate.
total, migrated = db.attachment_specs_online_data_migration(
self.ctxt, 50)
self.assertEqual(0, total)
self.assertEqual(0, migrated)
# We have to create a real volume because of the joinedload in the
# DB API query to get the volume attachment.
volume = db.volume_create(self.ctxt, {'host': 'host1'})
# Now create three volume attachments with specs and migrate them
# in batches of 2 to show we are enforcing the limit.
for x in range(3):
attachment = objects.VolumeAttachment(
self.ctxt, attach_status='attaching', volume_id=volume['id'])
attachment.create()
# Create an attachment_specs entry for the attachment.
db.attachment_specs_update_or_create(
self.ctxt, attachment.id, connector)
# Migrate 2 at a time.
total, migrated = db.attachment_specs_online_data_migration(
self.ctxt, 2)
self.assertEqual(3, total)
self.assertEqual(2, migrated)
# This should complete the migration.
total, migrated = db.attachment_specs_online_data_migration(
self.ctxt, 2)
self.assertEqual(1, total)
self.assertEqual(1, migrated)
# Run it one more time to make sure there is nothing left.
total, migrated = db.attachment_specs_online_data_migration(
self.ctxt, 2)
self.assertEqual(0, total)
self.assertEqual(0, migrated)