openstack
/
cinder
Code Issues Proposed changes

Merge "db: Remove final users of 'get_session'"

This commit is contained in:
Zuul 2022-06-09 17:04:18 +00:00 committed by Gerrit Code Review
parent cf786fbd37 c4a4c91ee6
commit 872b7fa3d9
5 changed files with 153 additions and 131 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

187
cinder/cmd/manage.py
View File

@ -372,54 +372,63 @@ class QuotaCommands(object):
"""
self._check_sync(project_id, do_fix=True)
def _get_quota_projects(self, ctxt, project_id):
@db_api.main_context_manager.reader
def _get_quota_projects(self, context, project_id):
"""Get project ids that have quota_usage entries."""
if project_id:
model = models.QuotaUsage
session = db_api.get_session()
# If the project does not exist
if not session.query(db_api.sql.exists().where(
db_api.and_(model.project_id == project_id,
~model.deleted))).scalar():
print('Project id %s has no quota usage. Nothing to do.' %
project_id)
if not context.session.query(
db_api.sql.exists()
.where(
db_api.and_(
model.project_id == project_id,
~model.deleted,
),
)
).scalar():
print(
'Project id %s has no quota usage. Nothing to do.' %
project_id,
)
return []
return [project_id]
projects = db_api.model_query(context,
models.QuotaUsage,
read_deleted="no").\
with_entities('project_id').\
distinct().\
all()
projects = db_api.model_query(
context,
models.QuotaUsage,
read_deleted="no"
).with_entities('project_id').distinct().all()
project_ids = [row.project_id for row in projects]
return project_ids
def _get_usages(self, ctxt, session, resources, project_id):
def _get_usages(self, context, resources, project_id):
"""Get data necessary to check out of sync quota usage.
Returns a list QuotaUsage instances for the specific project
"""
usages = db_api.model_query(ctxt,
db_api.models.QuotaUsage,
read_deleted="no",
session=session).\
filter_by(project_id=project_id).\
with_for_update().\
all()
usages = db_api.model_query(
context,
db_api.models.QuotaUsage,
read_deleted="no",
).filter_by(project_id=project_id).with_for_update().all()
return usages
def _get_reservations(self, ctxt, session, project_id, usage_id):
def _get_reservations(self, context, project_id, usage_id):
"""Get reservations for a given project and usage id."""
reservations = db_api.model_query(ctxt, models.Reservation,
read_deleted="no",
session=session).\
filter_by(project_id=project_id, usage_id=usage_id).\
with_for_update().\
all()
reservations = (
db_api.model_query(
context,
models.Reservation,
read_deleted="no",
)
.filter_by(project_id=project_id, usage_id=usage_id)
.with_for_update()
.all()
)
return reservations
def _check_duplicates(self, ctxt, session, usages, do_fix):
def _check_duplicates(self, context, usages, do_fix):
"""Look for duplicated quota used entries (bug#1484343)
If we have duplicates and we are fixing them, then we reassign the
@ -443,15 +452,17 @@ class QuotaCommands(object):
# Each of the duplicates can have reservations
reassigned = 0
for usage in resource_usages[1:]:
reservations = self._get_reservations(ctxt, session,
usage.project_id,
usage.id)
reservations = self._get_reservations(
context,
usage.project_id,
usage.id,
)
reassigned += len(reservations)
for reservation in reservations:
reservation.usage_id = keep_usage.id
keep_usage.in_use += usage.in_use
keep_usage.reserved += usage.reserved
usage.delete(session=session)
usage.delete(context.session)
print('duplicates removed & %s reservations reassigned' %
reassigned)
else:
@ -471,59 +482,83 @@ class QuotaCommands(object):
# projects removed will just turn nothing on the quota usage.
projects = self._get_quota_projects(ctxt, project_id)
session = db_api.get_session()
action_msg = ' - fixed' if do_fix else ''
discrepancy = False
# NOTE: It's important to always get the quota first and then the
# reservations to prevent deadlocks with quota commit and rollback from
# running Cinder services.
for project in projects:
with session.begin():
print('Processing quota usage for project %s' % project)
# We only want to sync existing quota usage rows
usages = self._get_usages(ctxt, session, resources, project)
discrepancy &= self._check_project_sync(
ctxt,
project,
do_fix,
resources,
)
# Check for duplicated entries (bug#1484343)
usages, duplicates_found = self._check_duplicates(ctxt,
session,
usages,
do_fix)
if duplicates_found:
discrepancy = True
# Check quota and reservations
for usage in usages:
resource_name = usage.resource
# Get the correct value for this quota usage resource
updates = db_api._get_sync_updates(ctxt, project, session,
resources,
resource_name)
in_use = updates[resource_name]
if in_use != usage.in_use:
print('\t%s: invalid usage saved=%s actual=%s%s' %
(resource_name, usage.in_use, in_use,
action_msg))
discrepancy = True
if do_fix:
usage.in_use = in_use
reservations = self._get_reservations(ctxt, session,
project, usage.id)
num_reservations = sum(r.delta for r in reservations
if r.delta > 0)
if num_reservations != usage.reserved:
print('\t%s: invalid reserved saved=%s actual=%s%s' %
(resource_name, usage.reserved,
num_reservations, action_msg))
discrepancy = True
if do_fix:
usage.reserved = num_reservations
print('Action successfully completed')
return discrepancy
@db_api.main_context_manager.reader
def _check_project_sync(self, context, project, do_fix, resources):
print('Processing quota usage for project %s' % project)
discrepancy = False
action_msg = ' - fixed' if do_fix else ''
# We only want to sync existing quota usage rows
usages = self._get_usages(context, resources, project)
# Check for duplicated entries (bug#1484343)
usages, duplicates_found = self._check_duplicates(
context, usages, do_fix,
)
if duplicates_found:
discrepancy = True
# Check quota and reservations
for usage in usages:
resource_name = usage.resource
# Get the correct value for this quota usage resource
updates = db_api._get_sync_updates(
context,
project,
resources,
resource_name,
)
in_use = updates[resource_name]
if in_use != usage.in_use:
print(
'\t%s: invalid usage saved=%s actual=%s%s' %
(resource_name, usage.in_use, in_use, action_msg)
)
discrepancy = True
if do_fix:
usage.in_use = in_use
reservations = self._get_reservations(
context,
project,
usage.id,
)
num_reservations = sum(
r.delta for r in reservations if r.delta > 0
)
if num_reservations != usage.reserved:
print(
'\t%s: invalid reserved saved=%s actual=%s%s' %
(
resource_name,
usage.reserved,
num_reservations,
action_msg,
)
)
discrepancy = True
if do_fix:
usage.reserved = num_reservations
return discrepancy
class VersionCommands(object):
"""Class for exposing the codebase version."""

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

@ -86,13 +86,7 @@ def configure(conf):
def get_engine():
return main_context_manager._factory.get_legacy_facade().get_engine()
def get_session(**kwargs):
return main_context_manager._factory.get_legacy_facade().get_session(
**kwargs
)
return main_context_manager.writer.get_engine()
def dispose_engine():

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

@ -100,21 +100,30 @@ class DBCommonFilterTestCase(BaseTest):
@mock.patch('sqlalchemy.orm.query.Query.filter')
def test__process_model_like_filter(self, mock_filter):
filters = {'display_name': 'fake_name',
'display_description': 'fake_description',
'host': 123,
'status': []}
session = sqlalchemy_api.get_session()
query = session.query(models.Volume)
filters = {
'display_name': 'fake_name',
'display_description': 'fake_description',
'host': 123,
'status': [],
}
with sqlalchemy_api.main_context_manager.writer.using(self.ctxt):
query = self.ctxt.session.query(models.Volume)
mock_filter.return_value = query
with mock.patch.object(operators.Operators, 'op') as mock_op:
def fake_operator(value):
return value
mock_op.return_value = fake_operator
sqlalchemy_api._process_model_like_filter(models.Volume,
query, filters)
calls = [call('%fake_description%'),
call('%fake_name%'), call('%123%')]
sqlalchemy_api._process_model_like_filter(
models.Volume, query, filters,
)
calls = [
call('%fake_description%'),
call('%fake_name%'),
call('%123%'),
]
mock_filter.assert_has_calls(calls, any_order=True)
@ddt.data({'handler': [db.volume_create, db.volume_get_all],
@ -2395,11 +2404,9 @@ class DBAPIVolumeTypeTestCase(BaseTest):
# NOTE(dulek): Bug 1496747 uncovered problems when deleting accesses
# with id column higher than 128. This is regression test for that
# case.
session = sqlalchemy_api.get_session()
vta.id = 150
vta.save(session=session)
session.close()
with sqlalchemy_api.main_context_manager.writer.using(self.ctxt):
vta.id = 150
vta.save(self.ctxt.session)
db.volume_type_access_remove(self.ctxt, vt['id'], 'fake_project')
vtas = db.volume_type_access_get_all(self.ctxt, vt['id'])
@ -2602,11 +2609,10 @@ class DBAPIReservationTestCase(BaseTest):
reservations = _quota_reserve(self.ctxt, project, volumes=2)
# Force a smaller reserved value in quota_usages table
session = sqlalchemy_api.get_session()
with session.begin():
vol_usage = db.quota_usage_get(self.ctxt, project, 'volumes')
vol_usage = db.quota_usage_get(self.ctxt, project, 'volumes')
with sqlalchemy_api.main_context_manager.writer.using(self.ctxt):
vol_usage.reserved -= 1
vol_usage.save(session=session)
vol_usage.save(self.ctxt.session)
# When committing 2 volumes from reserved to used reserved should not
# go from 1 to -1 but from 1 to 0, but in-use should still increase by
@ -2623,11 +2629,10 @@ class DBAPIReservationTestCase(BaseTest):
reservations = _quota_reserve(self.ctxt, project, volumes=-2)
# Force a smaller in_use than the one the reservation will decrease
session = sqlalchemy_api.get_session()
with session.begin():
vol_usage = db.quota_usage_get(self.ctxt, 'project1', 'volumes')
vol_usage = db.quota_usage_get(self.ctxt, 'project1', 'volumes')
with sqlalchemy_api.main_context_manager.writer.using(self.ctxt):
vol_usage.in_use = 1
vol_usage.save(session=session)
vol_usage.save(self.ctxt.session)
# When committing -2 volumes from reserved to in-use they should not
# make in-use go from 1 to -1, but from 1 to 0
@ -2663,11 +2668,10 @@ class DBAPIReservationTestCase(BaseTest):
reservations = _quota_reserve(self.ctxt, project, volumes=2)
# Force a smaller reserved value in quota_usages table
session = sqlalchemy_api.get_session()
with session.begin():
vol_usage = db.quota_usage_get(self.ctxt, project, 'volumes')
vol_usage = db.quota_usage_get(self.ctxt, project, 'volumes')
with sqlalchemy_api.main_context_manager.writer.using(self.ctxt):
vol_usage.reserved -= 1
vol_usage.save(session=session)
vol_usage.save(self.ctxt.session)
# When rolling back 2 volumes from reserved when there's only 1 in the
# quota usage's reserved field, reserved should not go from 1 to -1
@ -2698,11 +2702,10 @@ class DBAPIReservationTestCase(BaseTest):
_quota_reserve(self.ctxt, project, volumes=2)
# Force a smaller reserved value in quota_usages table
session = sqlalchemy_api.get_session()
with session.begin():
vol_usage = db.quota_usage_get(self.ctxt, project, 'volumes')
vol_usage = db.quota_usage_get(self.ctxt, project, 'volumes')
with sqlalchemy_api.main_context_manager.writer.using(self.ctxt):
vol_usage.reserved -= 1
vol_usage.save(session=session)
vol_usage.save(self.ctxt.session)
# When expiring 2 volumes from reserved when there's only 1 in the
# quota usage's reserved field, reserved should not go from 1 to -1
@ -3537,12 +3540,7 @@ class DBAPIGenericTestCase(BaseTest):
# one is not being used) to confirm that the DB exists subquery is
# properly formulated and doesn't result in multiple rows, as such
# case would raise an exception when converting the result to an
# scalar. This would happen if for example the query wasn't generated
# directly using get_session but using model_query like this:
# query = model_query(context, model,
# sql.exists().where(and_(*conditions)))
# Instead of what we do:
# query = get_session().query(sql.exists().where(and_(*conditions)))
# scalar.
db.volume_create(self.ctxt, {'id': fake.VOLUME_ID,
'volume_type_id': fake.VOLUME_TYPE_ID})
db.volume_create(self.ctxt, {'id': fake.VOLUME2_ID,
@ -3647,8 +3645,7 @@ class EngineFacadeTestCase(BaseTest):
self.project_id = fake.PROJECT_ID
self.context = context.RequestContext(self.user_id, self.project_id)
@mock.patch.object(sqlalchemy_api, 'get_session')
def test_use_single_context_session_writer(self, mock_get_session):
def test_use_single_context_session_writer(self):
# Checks that session in context would not be overwritten by
# annotation @sqlalchemy_api.main_context_manager.writer if annotation
# is used twice.
@ -3667,8 +3664,7 @@ class EngineFacadeTestCase(BaseTest):
parent_session, child_session = fake_parent_method(self.context)
self.assertEqual(parent_session, child_session)
@mock.patch.object(sqlalchemy_api, 'get_session')
def test_use_single_context_session_reader(self, mock_get_session):
def test_use_single_context_session_reader(self):
# Checks that session in context would not be overwritten by
# annotation @sqlalchemy_api.main_context_manager.reader if annotation
# is used twice.

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

@ -1392,9 +1392,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
self.usages_created = {}
self.reservations_created = {}
def fake_get_session():
return FakeSession()
def fake_get_quota_usages(context, project_id, resources=None):
return self.usages.copy()
@ -1418,8 +1415,6 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
return reservation_ref
self.mock_object(sqa_api, 'get_session',
fake_get_session)
self.mock_object(sqa_api, '_get_quota_usages',
fake_get_quota_usages)
self.mock_object(sqa_api, '_quota_usage_create',

6
cinder/tests/unit/test_volume_types.py
View File

@ -217,8 +217,10 @@ class VolumeTypeTestCase(test.TestCase):
def test_get_all_volume_types(self):
"""Ensures that all volume types can be retrieved."""
session = db_api.get_session()
total_volume_types = session.query(models.VolumeType).count()
with db_api.main_context_manager.writer.using(self.ctxt):
total_volume_types = self.ctxt.session.query(
models.VolumeType,
).count()
vol_types = volume_types.get_all_types(self.ctxt)
self.assertEqual(total_volume_types, len(vol_types))