openstack
/
cinder
Code Issues Proposed changes

Quota: Fix multiple race conditions 

This patch fixes 2 race condition areas and adds a comment on why we are
not fixing another 2.

Both race conditions that we fix could have been fixed with a
"with_for_update", but one of them is more effciently handled doing a
single query update instead of retrieving the data and deleting one by
one each row.

Change-Id: I2900c8b4c06ce1a4c2bd6bb5da5274883f6a01fb
This commit is contained in:
Gorka Eguileor 2021-03-15 18:30:13 +01:00
parent 935ddf7c72
commit bcb33db887
1 changed files with 24 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1338,6 +1338,13 @@ def _dict_with_usage_id(usages):
def reservation_commit(context, reservations, project_id=None):
session = get_session()
with session.begin():
# NOTE: There's a potential race condition window with
# reservation_expire, since _get_reservation_resources does not lock
# the rows, but we won't fix it because:
# - Minuscule chance of happening, since quota expiration is usually
# very high
# - Solution could create a DB lock on rolling upgrades since we need
# to reverse the order of locking the rows.
usages = _get_quota_usages(
context, session, project_id,
resources=_get_reservation_resources(session, context,
@ -1363,6 +1370,13 @@ def reservation_commit(context, reservations, project_id=None):
def reservation_rollback(context, reservations, project_id=None):
session = get_session()
with session.begin():
# NOTE: There's a potential race condition window with
# reservation_expire, since _get_reservation_resources does not lock
# the rows, but we won't fix it because:
# - Minuscule chance of happening, since quota expiration is usually
# very high
# - Solution could create a DB lock on rolling upgrades since we need
# to reverse the order of locking the rows.
usages = _get_quota_usages(
context, session, project_id,
resources=_get_reservation_resources(session, context,
@ -1398,32 +1412,23 @@ def quota_destroy_all_by_project(context, project_id, only_quotas=False):
"""
session = get_session()
with session.begin():
quotas = model_query(context, models.Quota, session=session,
read_deleted="no").\
model_query(context, models.Quota, session=session,
read_deleted="no").\
filter_by(project_id=project_id).\
all()
for quota_ref in quotas:
quota_ref.delete(session=session)
update(models.Quota.delete_values())
if only_quotas:
return
quota_usages = model_query(context, models.QuotaUsage,
session=session, read_deleted="no").\
model_query(context, models.QuotaUsage, session=session,
read_deleted="no").\
filter_by(project_id=project_id).\
all()
update(models.QuotaUsage.delete_values())
for quota_usage_ref in quota_usages:
quota_usage_ref.delete(session=session)
reservations = model_query(context, models.Reservation,
session=session, read_deleted="no").\
model_query(context, models.Reservation, session=session,
read_deleted="no").\
filter_by(project_id=project_id).\
all()
for reservation_ref in reservations:
reservation_ref.delete(session=session)
update(models.Reservation.delete_values())
@require_admin_context
@ -1435,6 +1440,7 @@ def reservation_expire(context):
results = model_query(context, models.Reservation, session=session,
read_deleted="no").\
filter(models.Reservation.expire < current_time).\
with_for_update().\
all()
if results: