diff options
author | Zuul <zuul@review.opendev.org> | 2023-05-05 20:12:21 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2023-05-05 20:12:21 +0000 |
commit | d893368356dc9ec6a02f0d081959de3e32e79669 (patch) | |
tree | 65c44040677ef0526105a0a6ad5f6b7bb0b6e569 | |
parent | 232a67f44414059e20bf5277aadc43ef0fae6931 (diff) | |
parent | c0af5b3b5ea89d3147adf1054625f29d5b01b309 (diff) | |
download | neutron-d893368356dc9ec6a02f0d081959de3e32e79669.tar.gz |
Merge "Reduce lock contention on subnets"
5 files changed, 82 insertions, 18 deletions
diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index dbbe94de7a..d80e7b3fc7 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -76,7 +76,7 @@ LOG = logging.getLogger(__name__) def _ensure_subnet_not_used(context, subnet_id): - models_v2.Subnet.lock_register( + models_v2.Subnet.write_lock_register( context, exc.SubnetInUse(subnet_id=subnet_id), id=subnet_id) try: registry.publish( diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index e734996900..9c245ae689 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -683,7 +683,7 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): msg = ('This subnet is being modified by another concurrent ' 'operation') for subnet in subnets: - subnet.lock_register( + subnet.read_lock_register( context, exc.SubnetInUse(subnet_id=subnet.id, reason=msg), id=subnet.id) subnet_dicts = [self._make_subnet_dict(subnet, context=context) diff --git a/neutron/db/migration/alembic_migrations/versions/2023.2/expand/93f394357a27_remove_in_use_on_subnets.py b/neutron/db/migration/alembic_migrations/versions/2023.2/expand/93f394357a27_remove_in_use_on_subnets.py new file mode 100644 index 0000000000..0e9ceb6071 --- /dev/null +++ b/neutron/db/migration/alembic_migrations/versions/2023.2/expand/93f394357a27_remove_in_use_on_subnets.py @@ -0,0 +1,42 @@ +# Copyright 2023 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +from alembic import op +import sqlalchemy as sa + + +"""remove in_use from subnet + +Revision ID: 93f394357a27 +Revises: fc153938cdc1 +Create Date: 2023-03-07 14:48:15.763633 + +""" + +# revision identifiers, used by Alembic. +revision = '93f394357a27' +down_revision = 'fc153938cdc1' + + +def upgrade(): + op.drop_column('subnets', 'in_use') + + +def expand_drop_exceptions(): + """Support dropping 'in_use' column in table 'subnets'""" + + return { + sa.Column: ['subnets.in_use'] + } diff --git a/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD b/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD index 961bd474e5..88323e719e 100644 --- a/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD +++ b/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD @@ -1 +1 @@ -fc153938cdc1 +93f394357a27 diff --git a/neutron/db/models_v2.py b/neutron/db/models_v2.py index 9a03b46763..8ab9707b17 100644 --- a/neutron/db/models_v2.py +++ b/neutron/db/models_v2.py @@ -33,25 +33,47 @@ from neutron.db import rbac_db_models class HasInUse(object): """NeutronBaseV2 mixin, to add the flag "in_use" to a DB model. - The content of this flag (boolean) parameter is not relevant. The goal of - this field is to be used in a write transaction to mark a DB register as - "in_use". Writing any value on this DB parameter will lock the container - register. At the end of the DB transaction, the DB engine will check if - this register was modified or deleted. In such case, the transaction will - fail and won't be committed. - - "lock_register" is the method to write the register "in_use" column. - Because the lifespan of this DB lock is the DB transaction, there isn't an - unlock method. The lock will finish once the transaction ends. + The goal of this class is to allow users lock specific database rows with + a shared or exclusive lock (without necessarily introducing a change in + the table itself). Having these locks allows the DB engine to prevent + concurrent modifications (e.g. the deletion of a resource while we are + currently adding a new dependency on the resource). + + "read_lock_register" takes a shared DB lock on the row specified by the + filters. The lock is automatically released once the transaction ends. + You can have any number of parallel read locks on the same DB row. But + you can not have any write lock in parallel. + + "write_lock_register" takes an exclusive DB lock on the row specified by + the filters. The lock is automatically released on transaction commit. + You may only have one write lock on each row at a time. It therefor + blocks all other read and write locks to this row. """ - in_use = sa.Column(sa.Boolean(), nullable=False, - server_default=sql.false(), default=False) @classmethod - def lock_register(cls, context, exception, **filters): + def write_lock_register(cls, context, exception, **filters): + # we use `with_for_update()` to include `FOR UPDATE` in the sql + # statement. + # we need to set `enable_eagerloads(False)` so that we do not try to + # load attached resources (e.g. standardattributes) as this breaks the + # `FOR UPDATE` statement. num_reg = context.session.query( - cls).filter_by(**filters).update({'in_use': True}) - if num_reg != 1: + cls).filter_by(**filters).enable_eagerloads( + False).with_for_update().first() + if num_reg is None: + raise exception + + @classmethod + def read_lock_register(cls, context, exception, **filters): + # we use `with_for_update(read=True)` to include `LOCK IN SHARE MODE` + # in the sql statement. + # we need to set `enable_eagerloads(False)` so that we do not try to + # load attached resources (e.g. standardattributes) as this breaks the + # `LOCK IN SHARE MODE` statement. + num_reg = context.session.query( + cls).filter_by(**filters).enable_eagerloads( + False).with_for_update(read=True).first() + if num_reg is None: raise exception |