openstack/cinder
Code Issues Proposed changes

Remove nested quota leftovers

Browse Source 
In change-id Ide2d53caf1bc5e3ba49f34b2f48de31abaf655d0 we removed the
nested quotas, but there are still some leftover code of it in cinder.

This code is useless, and in some cases not cheap, for example the call
to quota_allocated_get_all_by_project we do in every reservation.

This patch removes most of the remaining nested quota code and leaves
the DB structure changes for the next cycle, since removing them now
would break rolling upgrades.

Change-Id: Ibdbef651208c856ed75dd9af4dd711895e212910
This commit is contained in:
Gorka Eguileor 2021-03-02 19:40:13 +01:00
parent 85aee67dc7
commit 6005fc25fb
6 changed files with 35 additions and 226 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
cinder/api/contrib/quotas.py
View File

@ -140,8 +140,7 @@ class QuotaSetsController(wsgi.Controller):
return {'quota_set': self._get_quotas(context, target_project_id)}
def _get_quota_usage(self, quota_obj):
return (quota_obj.get('in_use', 0) + quota_obj.get('allocated', 0) +
quota_obj.get('reserved', 0))
return (quota_obj.get('in_use', 0) + quota_obj.get('reserved', 0))
def defaults(self, req, id):
context = req.environ['cinder.context']

26
cinder/db/api.py
View File

@ -1060,10 +1060,9 @@ def volume_glance_metadata_copy_from_volume_to_volume(context,
###################
def quota_create(context, project_id, resource, limit, allocated=0):
def quota_create(context, project_id, resource, limit):
"""Create a quota for the given project and resource."""
return IMPL.quota_create(context, project_id, resource, limit,
allocated=allocated)
return IMPL.quota_create(context, project_id, resource, limit)
def quota_get(context, project_id, resource):
@ -1076,21 +1075,6 @@ def quota_get_all_by_project(context, project_id):
return IMPL.quota_get_all_by_project(context, project_id)
def quota_allocated_get_all_by_project(context, project_id):
"""Retrieve all allocated quotas associated with a given project."""
return IMPL.quota_allocated_get_all_by_project(context, project_id)
def quota_allocated_update(context, project_id,
resource, allocated):
"""Update allocated quota to subprojects or raise if it does not exist.
:raises cinder.exception.ProjectQuotaNotFound:
"""
return IMPL.quota_allocated_update(context, project_id,
resource, allocated)
def quota_update(context, project_id, resource, limit):
"""Update a quota or raise if it does not exist."""
return IMPL.quota_update(context, project_id, resource, limit)
@ -1166,12 +1150,10 @@ def quota_usage_get_all_by_project(context, project_id):
def quota_reserve(context, resources, quotas, deltas, expire,
until_refresh, max_age, project_id=None,
is_allocated_reserve=False):
until_refresh, max_age, project_id=None):
"""Check quotas and create appropriate reservations."""
return IMPL.quota_reserve(context, resources, quotas, deltas, expire,
until_refresh, max_age, project_id=project_id,
is_allocated_reserve=is_allocated_reserve)
until_refresh, max_age, project_id=project_id)
def reservation_commit(context, reservations, project_id=None):

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

@ -856,16 +856,6 @@ def quota_get_all_by_project(context, project_id):
return result
@require_context
def quota_allocated_get_all_by_project(context, project_id, session=None):
rows = model_query(context, models.Quota, read_deleted='no',
session=session).filter_by(project_id=project_id).all()
result = {'project_id': project_id}
for row in rows:
result[row.resource] = row.allocated
return result
@require_context
def _quota_get_all_by_resource(context, resource, session=None):
rows = model_query(context, models.Quota,
@ -876,13 +866,11 @@ def _quota_get_all_by_resource(context, resource, session=None):
@require_context
def quota_create(context, project_id, resource, limit, allocated):
def quota_create(context, project_id, resource, limit):
quota_ref = models.Quota()
quota_ref.project_id = project_id
quota_ref.resource = resource
quota_ref.hard_limit = limit
if allocated:
quota_ref.allocated = allocated
session = get_session()
with session.begin():
@ -908,15 +896,6 @@ def quota_update_resource(context, old_res, new_res):
quota.resource = new_res
@require_admin_context
def quota_allocated_update(context, project_id, resource, allocated):
session = get_session()
with session.begin():
quota_ref = _quota_get(context, project_id, resource, session=session)
quota_ref.allocated = allocated
return quota_ref
@require_admin_context
def quota_destroy(context, project_id, resource):
session = get_session()
@ -1089,7 +1068,7 @@ def _quota_usage_create(context, project_id, resource, in_use, reserved,
def _reservation_create(context, uuid, usage, project_id, resource, delta,
expire, session=None, allocated_id=None):
expire, session=None):
usage_id = usage['id'] if usage else None
reservation_ref = models.Reservation()
reservation_ref.uuid = uuid
@ -1098,7 +1077,6 @@ def _reservation_create(context, uuid, usage, project_id, resource, delta,
reservation_ref.resource = resource
reservation_ref.delta = delta
reservation_ref.expire = expire
reservation_ref.allocated_id = allocated_id
reservation_ref.save(session=session)
return reservation_ref
@ -1149,8 +1127,7 @@ def quota_usage_update_resource(context, old_res, new_res):
@require_context
@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True)
def quota_reserve(context, resources, quotas, deltas, expire,
until_refresh, max_age, project_id=None,
is_allocated_reserve=False):
until_refresh, max_age, project_id=None):
elevated = context.elevated()
session = get_session()
with session.begin():
@ -1160,9 +1137,6 @@ def quota_reserve(context, resources, quotas, deltas, expire,
# Get the current usages
usages = _get_quota_usages(context, session, project_id,
resources=deltas.keys())
allocated = quota_allocated_get_all_by_project(context, project_id,
session=session)
allocated.pop('project_id')
# Handle usage refresh
work = set(deltas.keys())
@ -1242,12 +1216,8 @@ def quota_reserve(context, resources, quotas, deltas, expire,
usages[resource].until_refresh = until_refresh or None
# Check for deltas that would go negative
if is_allocated_reserve:
unders = [r for r, delta in deltas.items()
if delta < 0 and delta + allocated.get(r, 0) < 0]
else:
unders = [r for r, delta in deltas.items()
if delta < 0 and delta + usages[r].in_use < 0]
unders = [r for r, delta in deltas.items()
if delta < 0 and delta + usages[r].in_use < 0]
# TODO(mc_nair): Should ignore/zero alloc if using non-nested driver
@ -1258,7 +1228,7 @@ def quota_reserve(context, resources, quotas, deltas, expire,
# problems.
overs = [r for r, delta in deltas.items()
if quotas[r] >= 0 and delta >= 0 and
quotas[r] < delta + usages[r].total + allocated.get(r, 0)]
quotas[r] < delta + usages[r].total]
# NOTE(Vek): The quota check needs to be in the transaction,
# but the transaction doesn't fail just because
@ -1272,24 +1242,9 @@ def quota_reserve(context, resources, quotas, deltas, expire,
reservations = []
for resource, delta in deltas.items():
usage = usages[resource]
allocated_id = None
if is_allocated_reserve:
try:
quota = _quota_get(context, project_id, resource,
session=session)
except exception.ProjectQuotaNotFound:
# If we were using the default quota, create DB entry
quota = quota_create(context, project_id, resource,
quotas[resource], 0)
# Since there's no reserved/total for allocated, update
# allocated immediately and subtract on rollback if needed
quota_allocated_update(context, project_id, resource,
quota.allocated + delta)
allocated_id = quota.id
usage = None
reservation = _reservation_create(
elevated, str(uuid.uuid4()), usage, project_id, resource,
delta, expire, session=session, allocated_id=allocated_id)
delta, expire, session=session)
reservations.append(reservation.uuid)
@ -1305,15 +1260,14 @@ def quota_reserve(context, resources, quotas, deltas, expire,
#
# To prevent this, we only update the
# reserved value if the delta is positive.
if delta > 0 and not is_allocated_reserve:
if delta > 0:
usages[resource].reserved += delta
if unders:
LOG.warning("Change will make usage less than 0 for the following "
"resources: %s", unders)
if overs:
usages = {k: dict(in_use=v.in_use, reserved=v.reserved,
allocated=allocated.get(k, 0))
usages = {k: dict(in_use=v.in_use, reserved=v.reserved)
for k, v in usages.items()}
raise exception.OverQuota(overs=sorted(overs), quotas=quotas,
usages=usages)
@ -1361,12 +1315,10 @@ def reservation_commit(context, reservations, project_id=None):
usages = _dict_with_usage_id(usages)
for reservation in _quota_reservations(session, context, reservations):
# Allocated reservations will have already been bumped
if not reservation.allocated_id:
usage = usages[reservation.usage_id]
if reservation.delta >= 0:
usage.reserved -= reservation.delta
usage.in_use += reservation.delta
usage = usages[reservation.usage_id]
if reservation.delta >= 0:
usage.reserved -= reservation.delta
usage.in_use += reservation.delta
reservation.delete(session=session)
@ -1382,12 +1334,9 @@ def reservation_rollback(context, reservations, project_id=None):
reservations))
usages = _dict_with_usage_id(usages)
for reservation in _quota_reservations(session, context, reservations):
if reservation.allocated_id:
reservation.quota.allocated -= reservation.delta
else:
usage = usages[reservation.usage_id]
if reservation.delta >= 0:
usage.reserved -= reservation.delta
usage = usages[reservation.usage_id]
if reservation.delta >= 0:
usage.reserved -= reservation.delta
reservation.delete(session=session)
@ -1456,12 +1405,8 @@ def reservation_expire(context):
if results:
for reservation in results:
if reservation.delta >= 0:
if reservation.allocated_id:
reservation.quota.allocated -= reservation.delta
reservation.quota.save(session=session)
else:
reservation.usage.reserved -= reservation.delta
reservation.usage.save(session=session)
reservation.usage.reserved -= reservation.delta
reservation.usage.save(session=session)
reservation.delete(session=session)

3
cinder/db/sqlalchemy/models.py
View File

@ -616,6 +616,7 @@ class Quota(BASE, CinderBase):
resource = Column(String(255))
hard_limit = Column(Integer, nullable=True)
# TODO: (X release): Remove allocated, belonged to nested quotas
allocated = Column(Integer, default=0)
@ -670,6 +671,7 @@ class Reservation(BASE, CinderBase):
usage_id = Column(Integer, ForeignKey('quota_usages.id'), nullable=True,
index=True)
# TODO: (X release): Remove allocated_id, belonged to nested quotas
allocated_id = Column(Integer, ForeignKey('quotas.id'), nullable=True,
index=True)
@ -684,6 +686,7 @@ class Reservation(BASE, CinderBase):
foreign_keys=usage_id,
primaryjoin='and_(Reservation.usage_id == QuotaUsage.id,'
'QuotaUsage.deleted == False)')
# TODO: (X release): Remove allocated_id, belonged to nested quotas
quota = relationship(
"Quota",
foreign_keys=allocated_id,

15
cinder/quota.py
View File

@ -186,20 +186,16 @@ class DbQuotaDriver(object):
default value, if there is no value from the
quota class) will be reported if there is no
specific value for the resource.
:param usages: If True, the current in_use, reserved and allocated
counts will also be returned.
:param usages: If True, the current in_use and reserved counts will
also be returned.
"""
quotas = {}
project_quotas = db.quota_get_all_by_project(context, project_id)
allocated_quotas = None
default_quotas = None
if usages:
project_usages = db.quota_usage_get_all_by_project(context,
project_id)
allocated_quotas = db.quota_allocated_get_all_by_project(
context, project_id)
allocated_quotas.pop('project_id')
# Get the quotas for the appropriate class. If the project ID
# matches the one in the context, we use the quota_class from
@ -237,9 +233,6 @@ class DbQuotaDriver(object):
quotas[resource.name].update(
in_use=usage.get('in_use', 0),
reserved=usage.get('reserved', 0), )
if allocated_quotas:
quotas[resource.name].update(
allocated=allocated_quotas.get(resource.name, 0), )
return quotas
def _get_quotas(self, context, resources, keys, has_sync, project_id=None):
@ -684,8 +677,8 @@ class QuotaEngine(object):
default value, if there is no value from the
quota class) will be reported if there is no
specific value for the resource.
:param usages: If True, the current in_use, reserved and
allocated counts will also be returned.
:param usages: If True, the current in_use and reserved counts will
also be returned.
"""
return self._driver.get_project_quotas(context, self.resources,
project_id,

123
cinder/tests/unit/test_quota.py
View File

@ -888,20 +888,13 @@ class DbQuotaDriverBaseTestCase(test.TestCase):
)
# These can be used for expected defaults for child/non-child
self._default_quotas_non_child = dict(
self._default_quotas = dict(
volumes=10,
snapshots=10,
gigabytes=1000,
backups=10,
backup_gigabytes=1000,
per_volume_gigabytes=-1)
self._default_quotas_child = dict(
volumes=0,
snapshots=0,
gigabytes=0,
backups=0,
backup_gigabytes=0,
per_volume_gigabytes=0)
self.calls = []
@ -936,15 +929,6 @@ class DbQuotaDriverBaseTestCase(test.TestCase):
backup_gigabytes=500)
self.mock_object(db, 'quota_class_get_all_by_name', fake_qcgabn)
def _mock_allocated_get_all_by_project(self, allocated_quota=False):
def fake_qagabp(context, project_id, session=None):
self.calls.append('quota_allocated_get_all_by_project')
if allocated_quota:
return dict(project_id=project_id, volumes=3)
return dict(project_id=project_id)
self.mock_object(db, 'quota_allocated_get_all_by_project', fake_qagabp)
class DbQuotaDriverTestCase(DbQuotaDriverBaseTestCase):
def setUp(self):
@ -1020,14 +1004,12 @@ class DbQuotaDriverTestCase(DbQuotaDriverBaseTestCase):
def test_get_project_quotas(self):
self._mock_get_by_project()
self._mock_volume_type_get_all()
self._mock_allocated_get_all_by_project()
result = self.driver.get_project_quotas(
FakeContext('test_project', 'test_class'),
quota.QUOTAS.resources, 'test_project')
self.assertEqual(['quota_get_all_by_project',
'quota_usage_get_all_by_project',
'quota_allocated_get_all_by_project',
'quota_class_get_all_by_name',
'quota_class_get_defaults', ], self.calls)
self.assertEqual(dict(volumes=dict(limit=10,
@ -1054,7 +1036,7 @@ class DbQuotaDriverTestCase(DbQuotaDriverBaseTestCase):
@mock.patch('cinder.quota.db.quota_class_get_defaults')
def test_get_project_quotas_lazy_load_defaults(
self, mock_defaults, mock_quotas):
defaults = self._default_quotas_non_child
defaults = self._default_quotas
volume_types = volume.volume_types.get_all_types(
context.get_admin_context())
for vol_type in volume_types:
@ -1075,44 +1057,6 @@ class DbQuotaDriverTestCase(DbQuotaDriverBaseTestCase):
quota.QUOTAS.resources, 'test_project', usages=False)
self.assertTrue(mock_defaults.called)
def test_get_root_project_with_subprojects_quotas(self):
self._mock_get_by_project()
self._mock_volume_type_get_all()
self._mock_allocated_get_all_by_project(allocated_quota=True)
result = self.driver.get_project_quotas(
FakeContext('test_project', None),
quota.QUOTAS.resources, 'test_project')
self.assertEqual(['quota_get_all_by_project',
'quota_usage_get_all_by_project',
'quota_allocated_get_all_by_project',
'quota_class_get_defaults', ], self.calls)
self.assertEqual(dict(volumes=dict(limit=10,
in_use=2,
reserved=0,
allocated=3, ),
snapshots=dict(limit=10,
in_use=2,
reserved=0,
allocated=0, ),
gigabytes=dict(limit=50,
in_use=10,
reserved=0,
allocated=0, ),
backups=dict(limit=10,
in_use=2,
reserved=0,
allocated=0, ),
backup_gigabytes=dict(limit=50,
in_use=10,
reserved=0,
allocated=0, ),
per_volume_gigabytes=dict(in_use=0,
limit=-1,
reserved=0,
allocated=0)
), result)
def test_get_project_quotas_alt_context_no_class(self):
self._mock_get_by_project()
self._mock_volume_type_get_all()
@ -1455,11 +1399,10 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
return quota_usage_ref
def fake_reservation_create(context, uuid, usage_id, project_id,
resource, delta, expire, session=None,
allocated_id=None):
resource, delta, expire, session=None):
reservation_ref = self._make_reservation(
uuid, usage_id, project_id, resource, delta, expire,
timeutils.utcnow(), timeutils.utcnow(), allocated_id)
timeutils.utcnow(), timeutils.utcnow())
self.reservations_created[resource] = reservation_ref
@ -1518,7 +1461,7 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
(actual, value, resource))
def _make_reservation(self, uuid, usage_id, project_id, resource,
delta, expire, created_at, updated_at, alloc_id):
delta, expire, created_at, updated_at):
reservation_ref = sqa_models.Reservation()
reservation_ref.id = len(self.reservations_created)
reservation_ref.uuid = uuid
@ -1531,7 +1474,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
reservation_ref.updated_at = updated_at
reservation_ref.deleted_at = None
reservation_ref.deleted = False
reservation_ref.allocated_id = alloc_id
return reservation_ref
@ -1552,57 +1494,12 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
self.assertEqual(0, len(reservations))
def _mock_allocated_get_all_by_project(self, allocated_quota=False):
def fake_qagabp(context, project_id, session=None):
self.assertEqual('test_project', project_id)
self.assertIsNotNone(session)
if allocated_quota:
return dict(project_id=project_id, volumes=3,
gigabytes = 2 * 1024)
return dict(project_id=project_id)
self.mock_object(sqa_api, 'quota_allocated_get_all_by_project',
fake_qagabp)
def test_quota_reserve_with_allocated(self):
context = FakeContext('test_project', 'test_class')
# Allocated quota for volume will be updated for 3
self._mock_allocated_get_all_by_project(allocated_quota=True)
# Quota limited for volume updated for 10
quotas = dict(volumes=10,
gigabytes=10 * 1024, )
# Try reserve 7 volumes
deltas = dict(volumes=7,
gigabytes=2 * 1024, )
result = sqa_api.quota_reserve(context, self.resources, quotas,
deltas, self.expire, 5, 0)
# The reservation works
self.compare_reservation(
result,
[dict(resource='volumes',
usage_id=self.usages_created['volumes'],
project_id='test_project',
delta=7),
dict(resource='gigabytes',
usage_id=self.usages_created['gigabytes'],
delta=2 * 1024), ])
# But if we try reserve 8 volumes(more free quota that we have)
deltas = dict(volumes=8,
gigabytes=2 * 1024, )
self.assertRaises(exception.OverQuota,
sqa_api.quota_reserve,
context, self.resources, quotas,
deltas, self.expire, 0, 0)
def test_quota_reserve_create_usages(self):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5,
gigabytes=10 * 1024, )
deltas = dict(volumes=2,
gigabytes=2 * 1024, )
self._mock_allocated_get_all_by_project()
result = sqa_api.quota_reserve(context, self.resources, quotas,
deltas, self.expire, 0, 0)
@ -1636,7 +1533,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
gigabytes=10 * 1024, )
deltas = dict(volumes=2,
gigabytes=2 * 1024, )
self._mock_allocated_get_all_by_project()
result = sqa_api.quota_reserve(context, self.resources, quotas,
deltas, self.expire, 5, 0)
@ -1667,7 +1563,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5, gigabytes=10 * 1024, )
deltas = dict(volumes=2, gigabytes=2 * 1024, )
self._mock_allocated_get_all_by_project()
result = sqa_api.quota_reserve(context, self.resources, quotas,
deltas, self.expire, 5, 0)
@ -1701,7 +1596,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5, gigabytes=10 * 1024, )
deltas = dict(volumes=2, gigabytes=2 * 1024, )
self._mock_allocated_get_all_by_project()
# Simulate service is now running with until_refresh set to 5
sqa_api.quota_reserve(context, self.resources, quotas, deltas,
@ -1726,7 +1620,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5, gigabytes=10 * 1024, )
deltas = dict(volumes=2, gigabytes=2 * 1024, )
self._mock_allocated_get_all_by_project()
# Simulate service is now running with until_refresh disabled
sqa_api.quota_reserve(context, self.resources, quotas, deltas,
@ -1754,7 +1647,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5, gigabytes=10 * 1024, )
deltas = dict(volumes=2, gigabytes=2 * 1024, )
self._mock_allocated_get_all_by_project()
result = sqa_api.quota_reserve(context, self.resources, quotas,
deltas, self.expire, 0, max_age)
@ -1790,7 +1682,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5, gigabytes=10 * 1024, )
deltas = dict(volumes=2, gigabytes=2 * 1024, )
self._mock_allocated_get_all_by_project()
result = sqa_api.quota_reserve(context, self.resources, quotas,
deltas, self.expire, 0, max_age)
@ -1821,7 +1712,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5, gigabytes=10 * 1024, )
deltas = dict(volumes=2, gigabytes=2 * 1024, )
self._mock_allocated_get_all_by_project()
result = sqa_api.quota_reserve(context, self.resources, quotas,
deltas, self.expire, 0, 0)
@ -1852,7 +1742,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5, gigabytes=10 * 1024, )
deltas = dict(volumes=-2, gigabytes=-2 * 1024, )
self._mock_allocated_get_all_by_project()
result = sqa_api.quota_reserve(context, self.resources, quotas,
deltas, self.expire, 0, 0)
@ -1883,7 +1772,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5, gigabytes=10 * 1024, )
deltas = dict(volumes=2, gigabytes=2 * 1024, )
self._mock_allocated_get_all_by_project()
self.assertRaises(exception.OverQuota,
sqa_api.quota_reserve,
context, self.resources, quotas,
@ -1909,7 +1797,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
context = FakeContext('test_project', 'test_class')
quotas = dict(volumes=5, gigabytes=10 * 1024, )
deltas = dict(volumes=-2, gigabytes=-2 * 1024, )
self._mock_allocated_get_all_by_project()
result = sqa_api.quota_reserve(context, self.resources, quotas,
deltas, self.expire, 0, 0)