Fix DetachedInstanceError
This work is part of Debian integration effort. Sysinv interacts with a postgres database. Sysinv retrieves objects through oslo.db + sqlalchemy. On CentOS 7 objects were attached to a session, but not anymore. Trying to read lazy joined fields will result in an exception thrown. This will prevent the bootstrap from finishing on Debian system. The fix is to make sure database objects are attached to a session. Transforming database info to sysinv objects always had a dependency on the database ORM. Now decoupling sysinv objects from database management objects: create the concept of sysinv database object, such that there is no need for sysinv object to know about database sessions. (Think how sysinv.objects needed to know about database sessions for objectify function in the first place, even though this was hidden on CentOS 7.) A commit responsible for the different behavior between CentOS and Debian was not pinpointed. Developer notes: CentOS 7 vs Debian Bullseye Python 2.7 Python 3.9 Postgresql 9 Postgresql 13 eventlet==0.24.1 eventlet==0.26.1 oslo.db==4.40.0 oslo.db==8.4.0 SQLAlchemy==1.1.11 SQLAlchemy==1.3.22 sqlalchemy-migrate==0.11.0 sqlalchemy-migrate==0.13.0 sqlparse==0.1.18 sqlparse==0.4.1 MySQL-python==1.2.5 mysqlclient==1.4.4 Downgrading/upgrading SQLAlchemy version on live Debian controller didn't have any impact. There are ~100 oslo.db commits between versions, parsing code changes reveals no obvious connection. There are ~30 sqlalchemy-migrate commits between versions, parsing code changes reveals no obvious connection. There are ~600 sqlparse commits between versions, but shouldn't be responsible. I don't think it is worth further pursuing a root commit now, since the issue is well understood, but not when it started to occur. Performance impact: These are not accurate tests, but enough to determine the magnitude of performance hit. I was using VMs, host cpus not isolated, cpus not pinned to VM, 8 vCPUs, 15GB RAM. Performance hit is of 10^-4 numbers on a few snippets on controller, both CentOS and Debian. In some cases when running unit tests(on host), a slowdown of 20ms was observed, but most of the time the impact was lower than 1ms. This provides great insight as there are hundreds of queries to the DB. Testing shows slow ops are not due to python overhead introduced for expection paths. Tested py27, py36, py39 tox targets. CentOS 7 tests(AIO-SX VM IPv4 and AIO-SX HW IPv6): - PASS: build package and build image - PASS: AIO-SX bootstrap - PASS: AIO-SX Unlocked/Enabled/Available - PASS: platform-integ-app auto-applied - PASS: visual inspection of sysinv.log shows nothing unusual Debian Bullseye tests: - PASS: build package and build image - PASS: bootstrap goes past the breaking point Story: 2009101 Task: 44360 Signed-off-by: Dan Voiculeasa <dan.voiculeasa@windriver.com> Change-Id: If0c14dad03aad613a950dded2ad80565f0021164
This commit is contained in:
parent
211194beef
commit
697c810baa
File diff suppressed because it is too large
Load Diff
|
@ -0,0 +1,57 @@
|
|||
#
|
||||
# Copyright (c) 2022 Wind River Systems, Inc.
|
||||
#
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
#
|
||||
import functools
|
||||
|
||||
import eventlet
|
||||
from oslo_db.sqlalchemy import enginefacade
|
||||
from sqlalchemy.exc import InvalidRequestError
|
||||
from sqlalchemy.orm.exc import UnmappedInstanceError
|
||||
|
||||
ALREADY_ATTACHED_STRING = 'already attached'
|
||||
|
||||
|
||||
def _session_for_read():
|
||||
_context = eventlet.greenthread.getcurrent()
|
||||
return enginefacade.reader.using(_context)
|
||||
|
||||
|
||||
def objectify(klass):
|
||||
"""Decorator to convert database results into specified objects.
|
||||
:param klass: database results class
|
||||
"""
|
||||
|
||||
def the_decorator(fn):
|
||||
@functools.wraps(fn)
|
||||
def wrapper(*args, **kwargs):
|
||||
first_result = fn(*args, **kwargs)
|
||||
|
||||
with _session_for_read() as session:
|
||||
bound_session = True
|
||||
try:
|
||||
session.add(first_result)
|
||||
except UnmappedInstanceError:
|
||||
bound_session = False
|
||||
except InvalidRequestError as e:
|
||||
if ALREADY_ATTACHED_STRING in str(e):
|
||||
bound_session = False
|
||||
else:
|
||||
raise e
|
||||
|
||||
try:
|
||||
second_result = klass.from_db_object(first_result)
|
||||
except TypeError:
|
||||
# TODO(deva): handle lists of objects better
|
||||
# once support for those lands and is imported.
|
||||
second_result = [klass.from_db_object(obj) for obj in first_result]
|
||||
|
||||
if bound_session:
|
||||
session.expunge_all()
|
||||
|
||||
return second_result
|
||||
|
||||
return wrapper
|
||||
|
||||
return the_decorator
|
|
@ -12,12 +12,9 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
#
|
||||
# Copyright (c) 2013-2021 Wind River Systems, Inc.
|
||||
# Copyright (c) 2013-2022 Wind River Systems, Inc.
|
||||
#
|
||||
|
||||
|
||||
import functools
|
||||
|
||||
from sysinv.objects import address
|
||||
from sysinv.objects import address_mode
|
||||
from sysinv.objects import address_pool
|
||||
|
@ -109,27 +106,6 @@ from sysinv.objects import kube_rootca_update
|
|||
from sysinv.objects import kube_rootca_host_update
|
||||
|
||||
|
||||
def objectify(klass):
|
||||
"""Decorator to convert database results into specified objects.
|
||||
:param klass: database results class
|
||||
"""
|
||||
|
||||
def the_decorator(fn):
|
||||
@functools.wraps(fn)
|
||||
def wrapper(*args, **kwargs):
|
||||
result = fn(*args, **kwargs)
|
||||
try:
|
||||
return klass.from_db_object(result)
|
||||
except TypeError:
|
||||
# TODO(deva): handle lists of objects better
|
||||
# once support for those lands and is imported.
|
||||
return [klass.from_db_object(obj) for obj in result]
|
||||
|
||||
return wrapper
|
||||
|
||||
return the_decorator
|
||||
|
||||
|
||||
# alias objects for RPC compatibility
|
||||
ihost = host.ihost
|
||||
ilvg = lvg.LVG
|
||||
|
@ -286,7 +262,6 @@ __all__ = ("system",
|
|||
"tpmconfig",
|
||||
"tpmdevice",
|
||||
"certificate",
|
||||
"objectify",
|
||||
"storage_file",
|
||||
"storage_external",
|
||||
"storage_ceph_rook",
|
||||
|
@ -311,5 +286,4 @@ __all__ = ("system",
|
|||
"kube_rootca_update",
|
||||
# alias objects for RPC compatibility
|
||||
"ihost",
|
||||
"ilvg",
|
||||
"objectify")
|
||||
"ilvg")
|
||||
|
|
|
@ -1,8 +1,7 @@
|
|||
# vim: tabstop=4 shiftwidth=4 softtabstop=4
|
||||
# coding=utf-8
|
||||
#
|
||||
#
|
||||
# Copyright (c) 2013-2014 Wind River Systems, Inc.
|
||||
# Copyright (c) 2013-2022 Wind River Systems, Inc.
|
||||
#
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
#
|
||||
|
@ -11,6 +10,7 @@ import mock
|
|||
|
||||
from sysinv.db import api as db_api
|
||||
from sysinv.db.sqlalchemy import models
|
||||
from sysinv.db.sqlalchemy import objects as db_objects
|
||||
from sysinv import objects
|
||||
from sysinv.tests.db import base
|
||||
from sysinv.tests.db import utils
|
||||
|
@ -61,7 +61,7 @@ class TestHostObject(base.DbTestCase):
|
|||
|
||||
def test_objectify(self):
|
||||
|
||||
@objects.objectify(objects.host)
|
||||
@db_objects.objectify(objects.host)
|
||||
def _convert_db_node():
|
||||
return self._get_db_node(self.fake_node)
|
||||
|
||||
|
@ -75,7 +75,7 @@ class TestHostObject(base.DbTestCase):
|
|||
nodes.append(self._get_db_node(self.fake_node))
|
||||
return nodes
|
||||
|
||||
@objects.objectify(objects.host)
|
||||
@db_objects.objectify(objects.host)
|
||||
def _convert_db_nodes():
|
||||
return _get_db_nodes()
|
||||
|
||||
|
|
|
@ -1,8 +1,7 @@
|
|||
# vim: tabstop=4 shiftwidth=4 softtabstop=4
|
||||
# coding=utf-8
|
||||
#
|
||||
#
|
||||
# Copyright (c) 2019 Wind River Systems, Inc.
|
||||
# Copyright (c) 2019-2022 Wind River Systems, Inc.
|
||||
#
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
#
|
||||
|
@ -11,6 +10,7 @@ import mock
|
|||
|
||||
from sysinv.db import api as db_api
|
||||
from sysinv.db.sqlalchemy import models
|
||||
from sysinv.db.sqlalchemy import objects as db_objects
|
||||
from sysinv import objects
|
||||
from sysinv.tests.db import base
|
||||
from sysinv.tests.db import utils
|
||||
|
@ -64,7 +64,7 @@ class TestKubeHostUpgradesObject(base.DbTestCase):
|
|||
|
||||
def test_objectify(self):
|
||||
|
||||
@objects.objectify(objects.kube_host_upgrade)
|
||||
@db_objects.objectify(objects.kube_host_upgrade)
|
||||
def _convert_db_data():
|
||||
return self._get_db_data(self.fake_upgrade_data)
|
||||
|
||||
|
@ -79,7 +79,7 @@ class TestKubeHostUpgradesObject(base.DbTestCase):
|
|||
data.append(self._get_db_data(self.fake_upgrade_data))
|
||||
return data
|
||||
|
||||
@objects.objectify(objects.kube_host_upgrade)
|
||||
@db_objects.objectify(objects.kube_host_upgrade)
|
||||
def _convert_db_data_many():
|
||||
return _get_db_data_many()
|
||||
|
||||
|
|
|
@ -1,8 +1,7 @@
|
|||
# vim: tabstop=4 shiftwidth=4 softtabstop=4
|
||||
# coding=utf-8
|
||||
#
|
||||
#
|
||||
# Copyright (c) 2019 Wind River Systems, Inc.
|
||||
# Copyright (c) 2019-2022 Wind River Systems, Inc.
|
||||
#
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
#
|
||||
|
@ -11,6 +10,7 @@ import mock
|
|||
|
||||
from sysinv.db import api as db_api
|
||||
from sysinv.db.sqlalchemy import models
|
||||
from sysinv.db.sqlalchemy import objects as db_objects
|
||||
from sysinv import objects
|
||||
from sysinv.tests.db import base
|
||||
from sysinv.tests.db import utils
|
||||
|
@ -62,7 +62,7 @@ class TestKubeUpgradesObject(base.DbTestCase):
|
|||
|
||||
def test_objectify(self):
|
||||
|
||||
@objects.objectify(objects.kube_upgrade)
|
||||
@db_objects.objectify(objects.kube_upgrade)
|
||||
def _convert_db_data():
|
||||
return self._get_db_data(self.fake_upgrade_data)
|
||||
|
||||
|
@ -77,7 +77,7 @@ class TestKubeUpgradesObject(base.DbTestCase):
|
|||
data.append(self._get_db_data(self.fake_upgrade_data))
|
||||
return data
|
||||
|
||||
@objects.objectify(objects.kube_upgrade)
|
||||
@db_objects.objectify(objects.kube_upgrade)
|
||||
def _convert_db_data_many():
|
||||
return _get_db_data_many()
|
||||
|
||||
|
|
Loading…
Reference in New Issue