openstack/cinder
Code Issues Proposed changes

Add atomic conditional updates to objects

Browse Source 
To allow atomic state changes across Cinder services we need to
implement a way to easily do compare-and-swap.

This patch adds methods to allow compare-and-swap on DB models and on
Cinder Versioned Objects as well.

Conditions for the compare part of the update can consist of:
- Inclusion: status == 'available'
- Exclusion: status != 'in-use'
- Multi-inclusion: status in ('available', 'error')
- Multi-exclusion: status not in ('attaching', 'in-use')
- Sqlalchemy filters

A complete example of usage would be the compare-and-swap used in volume
delete requests that has to take in consideration not only the status
but the attach and migration status as well as the volume not having
snapshots:

 now = timeutils.utcnow()
 expected = {'attach_status': db.Not('attached'),
             'migration_status': None,
             'consistencygroup_id': None}
 good_status = ('available', 'error', 'error_restoring',
                'error_extending')
 if not force:
     expected.update(status=good_status)

 # Volume cannot have snapshots if we want to delete it
 filters = [~sql.exists().where(models.Volume.id ==
                                models.Snapshot.volume_id)]

 updated = vol_obj.conditional_update(
     {'status': 'deleting',
      'previous_status': vol_obj.model.status,
      'terminated_at': now},
     expected,
     filters)

It can also be specified whether to save already dirtied fields from the
objects or not and by default (if no expected_values argument is
provided) it will make sure that the entry in the DB has the same values
as the objects we are saving.

We can select values based on conditions using Case objects in the
'values' argument. For example:

 has_snapshot_filter = sql.exists().where(
     models.Snapshot.volume_id == models.Volume.id)
 case_values = volume.Case([(has_snapshot_filter, 'has-snapshot')],
                           else_='no-snapshot')
 volume.conditional_update({'status': case_values},
                           {'status': 'available'})

Exclusion and multi-exclusion will handle, by default, NULL values like
Python does instead of like SQL does, so NULL values will be considered
different than any non NULL values.  That way if we search for something
not equal to 1 we will also get NULL values.

WARNING: SQLAlchemy does not allow selecting order of SET clauses, so
for now we cannot do things like
    {'previous_status': model.status, 'status': 'retyping'}
because it will result in both previous_status and status being set to
'retyping'.  Issue has been reported [1] and a patch to fix it [2] has
been submitted.
[1]: https://bitbucket.org/zzzeek/sqlalchemy/issues/3541
[2]: https://github.com/zzzeek/sqlalchemy/pull/200

Specs: https://review.openstack.org/232599/

Implements: blueprint cinder-volume-active-active-support
Related-Bug: #1490944
Related-Bug: #1238093
Related-Bug: #1490946
Related-Bug: #1469659
Related-Bug: #1493120
Related-Bug: #1493419
Related-Bug: #1493476
Related-Bug: #1494466
Change-Id: If90a37f8c7d6fad8fc1f861d52ba862875920cdc
This commit is contained in:
Gorka Eguileor
2015-07-25 12:34:28 +02:00
parent 3754b3b063
commit 1070c28774
6 changed files with 780 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -19,6 +19,7 @@
"""Implementation of SQLAlchemy backend."""
import collections
import datetime as dt
import functools
import re
@@ -38,7 +39,7 @@ import osprofiler.sqlalchemy
import six
import sqlalchemy
from sqlalchemy import MetaData
from sqlalchemy import or_
from sqlalchemy import or_, case
from sqlalchemy.orm import joinedload, joinedload_all
from sqlalchemy.orm import RelationshipProperty
from sqlalchemy.schema import Table
@@ -50,13 +51,13 @@ from sqlalchemy.sql import sqltypes
from cinder.api import common
from cinder.common import sqlalchemyutils
from cinder import db
from cinder.db.sqlalchemy import models
from cinder import exception
from cinder.i18n import _, _LW, _LE, _LI
CONF = cfg.CONF
CONF.import_group("profiler", "cinder.service")
LOG = logging.getLogger(__name__)
options.set_defaults(CONF, connection='sqlite:///$state_path/cinder.sqlite')
@@ -75,6 +76,11 @@ def _create_facade_lazily():
**dict(CONF.database)
)
# NOTE(geguileo): To avoid a cyclical dependency we import the
# group here. Dependency cycle is objects.base requires db.api,
# which requires db.sqlalchemy.api, which requires service which
# requires objects.base
CONF.import_group("profiler", "cinder.service")
if CONF.profiler.profiler_enabled:
if CONF.profiler.trace_sqlalchemy:
osprofiler.sqlalchemy.add_tracing(sqlalchemy,
@@ -4191,3 +4197,84 @@ def get_by_id(context, model, id, *args, **kwargs):
_GET_METHODS[model] = _get_get_method(model)
return _GET_METHODS[model](context, id, *args, **kwargs)
def condition_db_filter(model, field, value):
"""Create matching filter.
If value is an iterable other than a string, any of the values is
a valid match (OR), so we'll use SQL IN operator.
If it's not an iterator == operator will be used.
"""
orm_field = getattr(model, field)
# For values that must match and are iterables we use IN
if (isinstance(value, collections.Iterable) and
not isinstance(value, six.string_types)):
# We cannot use in_ when one of the values is None
if None not in value:
return orm_field.in_(value)
return or_(orm_field == v for v in value)
# For values that must match and are not iterables we use ==
return orm_field == value
def condition_not_db_filter(model, field, value, auto_none=True):
"""Create non matching filter.
If value is an iterable other than a string, any of the values is
a valid match (OR), so we'll use SQL IN operator.
If it's not an iterator == operator will be used.
If auto_none is True then we'll consider NULL values as different as well,
like we do in Python and not like SQL does.
"""
result = ~condition_db_filter(model, field, value)
if (auto_none
and ((isinstance(value, collections.Iterable) and
not isinstance(value, six.string_types)
and None not in value)
or (value is not None))):
orm_field = getattr(model, field)
result = or_(result, orm_field.is_(None))
return result
def is_orm_value(obj):
"""Check if object is an ORM field or expression."""
return isinstance(obj, (sqlalchemy.orm.attributes.InstrumentedAttribute,
sqlalchemy.sql.expression.ColumnElement))
@_retry_on_deadlock
@require_context
def conditional_update(context, model, values, expected_values, filters=(),
include_deleted='no', project_only=False):
"""Compare-and-swap conditional update SQLAlchemy implementation."""
# Provided filters will become part of the where clause
where_conds = list(filters)
# Build where conditions with operators ==, !=, NOT IN and IN
for field, condition in expected_values.items():
if not isinstance(condition, db.Condition):
condition = db.Condition(condition, field)
where_conds.append(condition.get_filter(model, field))
# Transform case values
values = {field: case(value.whens, value.value, value.else_)
if isinstance(value, db.Case)
else value
for field, value in values.items()}
query = model_query(context, model, read_deleted=include_deleted,
project_only=project_only)
# Return True if we were able to change any DB entry, False otherwise
result = query.filter(*where_conds).update(values,
synchronize_session=False)
return 0 != result