UT failing with sqlalchemy 1.4

Bug #1926399 reported by Slawek Kaplonski
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Cinder
Undecided
Unassigned
OpenStack Compute (nova)
Undecided
Unassigned
OpenStack Shared File Systems Service (Manila)
High
Goutham Pacha Ravi
masakari
Critical
Radosław Piliszek
neutron
Critical
Rodolfo Alonso
oslo.db
Undecided
Unassigned
Revision history for this message
Radosław Piliszek (yoctozepto) wrote :

I appropriated this bug report to track progress of fixing.
We need oslo help anyway.

Changed in oslo.db:
status: New → Confirmed
Changed in masakari:
status: New → Triaged
summary: - Neutron UT failing with sqlalchemy 1.4
+ UT failing with sqlalchemy 1.4
Changed in masakari:
importance: Undecided → Critical
Changed in neutron:
assignee: nobody → Rodolfo Alonso (rodolfo-alonso-hernandez)
Revision history for this message
Rodolfo Alonso (rodolfo-alonso-hernandez) wrote :

Hello:

In [1] we optimized the SG rule retrieval. The rules are now loaded dynamically when needed. That means the DB object is retrieved but the "rules" field is only queried from the DB when the value is needed. That saves time most of the times if the code doesn't need to access to this value.

Those dynamically loaded values should not be added to the OVO classes. The generated value is a Query object, not the expected value. This should be considered as a synthetic value.

Regards.

[1]https://review.opendev.org/c/openstack/neutron/+/637407

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/788463

Changed in neutron:
status: Confirmed → In Progress
Changed in nova:
status: New → Triaged
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/788463
Committed: https://opendev.org/openstack/neutron/commit/de295f036df2cedec9de4c25f344ebb770f66e0a
Submitter: "Zuul (22348)"
Branch: master

commit de295f036df2cedec9de4c25f344ebb770f66e0a
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Wed Apr 28 09:36:07 2021 +0000

    Do not include dynamically loaded values into OVO testing

    Those OVO values loaded dynamically (for example [1]) should not be
    added to the tested object. A dynamic column in a DB register is a
    ``sqlalchemy.orm.AppenderQuery`` object that represents the needed
    query to retrieve this value.

    These OVO fields, mapped from the DB object, should be considered as
    synthetic fields and not be included in
    ``_BaseObjectTestCase.obj_fields``.

    [1]https://github.com/openstack/neutron/blob/ff2464bf33f1c84bd8525eca68c5774f5e14f1f1/neutron/db/models/securitygroup.py#L103-L106

    Closes-Bug: #1926399

    Change-Id: I4b59dbc71316eede52b22c0788de5fd682fbff30

Changed in neutron:
status: In Progress → Fix Released
Revision history for this message
Bernard Cafarelli (bcafarel) wrote :

Looking good for neutron with this fix in DNM https://review.opendev.org/c/openstack/neutron/+/789370 (only known nova failures in larger tempest tests)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to masakari (master)

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/masakari/+/790216

Changed in masakari:
status: Triaged → In Progress
Revision history for this message
Radosław Piliszek (yoctozepto) wrote :

Cinder team, please respond.

Changed in masakari:
assignee: nobody → Radosław Piliszek (yoctozepto)
Changed in cinder:
status: New → Confirmed
Changed in oslo.db:
status: Confirmed → Fix Released
Changed in nova:
status: Triaged → In Progress
Revision history for this message
Radosław Piliszek (yoctozepto) wrote :
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Cinder is seeing two types of error with 1.4.13 (current proposed u-c) that persist in 1.4.15 (latest SQLAlchemy release):

conditional_update broken: https://bugs.launchpad.net/cinder/+bug/1928083

calculate_resource_count broken: https://bugs.launchpad.net/cinder/+bug/1928085

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Fixes for the bugs mentioned in comment #9 have merged.

Changed in cinder:
status: Confirmed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to masakari (master)

Reviewed: https://review.opendev.org/c/openstack/masakari/+/790216
Committed: https://opendev.org/openstack/masakari/commit/e5246e282f7fd448f45ca096686974b6173e0850
Submitter: "Zuul (22348)"
Branch: master

commit e5246e282f7fd448f45ca096686974b6173e0850
Author: Radosław Piliszek <email address hidden>
Date: Fri May 7 10:12:52 2021 +0000

    Make unit tests compatible with SQLAlchemy 1.4

    The .count() method is gone from the Table class.
    Let's use func.count in its stead.

    Closes-Bug: #1926399
    Change-Id: Ica81271fe85478fdffae4ea0b6f4ed3b168f4552

Changed in masakari:
status: In Progress → Fix Released
Changed in manila:
importance: Undecided → High
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to manila (master)

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/manila/+/792059

Changed in manila:
status: New → In Progress
Revision history for this message
Goutham Pacha Ravi (gouthamr) wrote :

In Manila, we're using with_entities in a few places alongside loading options, which seems to have an issue: https://github.com/sqlalchemy/sqlalchemy/issues/6253

Changed in manila:
assignee: nobody → Goutham Pacha Ravi (gouthamr)
milestone: none → xena-1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/manila/+/792303

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to manila (master)

Reviewed: https://review.opendev.org/c/openstack/manila/+/792059
Committed: https://opendev.org/openstack/manila/commit/cadfe04b1f4a8abb12b7b652b187451710a73b81
Submitter: "Zuul (22348)"
Branch: master

commit cadfe04b1f4a8abb12b7b652b187451710a73b81
Author: Goutham Pacha Ravi <email address hidden>
Date: Tue May 18 18:58:16 2021 -0700

    Replace deprecated SQLAlchemy "with_lockmode"

    This method was deprecated in sqlalchemy 0.9 [1][2]
    and finally removed in version 1.4, which we'll
    be adopting very soon [3]. No end-user impact is
    expected, we're using a rudimentary form of the
    new "with_for_update" method.

    [1] https://docs.sqlalchemy.org/en/13/changelog/migration_09.html?highlight=with_lockmode#new-for-update-support-on-select-query
    [2] https://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.with_lockmode
    [3] http://lists.openstack.org/pipermail/openstack-discuss/2021-April/022094.html

    Change-Id: I0a3a89bd0741f6f91c39baaa68149551d8f4ce54
    Partial-Bug: #1926399
    Signed-off-by: Goutham Pacha Ravi <email address hidden>

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to manila (stable/wallaby)

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/manila/+/792295

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to manila (master)

Reviewed: https://review.opendev.org/c/openstack/manila/+/792303
Committed: https://opendev.org/openstack/manila/commit/6ace54c2601770a0e06418ce5b98c756c86f6a51
Submitter: "Zuul (22348)"
Branch: master

commit 6ace54c2601770a0e06418ce5b98c756c86f6a51
Author: Goutham Pacha Ravi <email address hidden>
Date: Wed May 19 17:14:24 2021 -0700

    Fix with_entities usage in db queries

    In order to retrieve shares with filters
    pertaining to separate tables (shares, instances, metadata,
    etc), we need to perform joined loads of those
    respective database tables as necessary. After the
    join, the query cannot select entities due to
    Github issue #6253 - The following is an error when
    the pattern is used:

    sqlalchemy.exc.ArgumentError: Query has only expression-based entities - can't find property named "share_metadata".

    sqlalchemy 1.4 has a performance improvement that
    delays query processing [2] and disallows this
    pattern of usage; we can use a query.count() instead.

    In another instance, we can perform joins selectively
    only if asked for.

    [1] https://github.com/sqlalchemy/sqlalchemy/issues/625
    [2] https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#many-core-and-orm-statement-objects-now-perform-much-of-their-construction-and-validation-in-the-compile-phase

    Change-Id: I8aa196c171bbc224cec06f517ea22c4e91cbc06a
    Closes-Bug: #1926399
    Signed-off-by: Goutham Pacha Ravi <email address hidden>

Changed in manila:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to manila (stable/wallaby)

Reviewed: https://review.opendev.org/c/openstack/manila/+/792295
Committed: https://opendev.org/openstack/manila/commit/3532f5d65432fab4d21fe2499474801104f9638e
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 3532f5d65432fab4d21fe2499474801104f9638e
Author: Goutham Pacha Ravi <email address hidden>
Date: Tue May 18 18:58:16 2021 -0700

    Replace deprecated SQLAlchemy "with_lockmode"

    This method was deprecated in sqlalchemy 0.9 [1][2]
    and finally removed in version 1.4, which we'll
    be adopting very soon [3]. No end-user impact is
    expected, we're using a rudimentary form of the
    new "with_for_update" method.

    [1] https://docs.sqlalchemy.org/en/13/changelog/migration_09.html?highlight=with_lockmode#new-for-update-support-on-select-query
    [2] https://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.with_lockmode
    [3] http://lists.openstack.org/pipermail/openstack-discuss/2021-April/022094.html

    Change-Id: I0a3a89bd0741f6f91c39baaa68149551d8f4ce54
    Partial-Bug: #1926399
    Signed-off-by: Goutham Pacha Ravi <email address hidden>
    (cherry picked from commit cadfe04b1f4a8abb12b7b652b187451710a73b81)

tags: added: in-stable-wallaby
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to manila (stable/wallaby)

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/manila/+/792663

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to manila (stable/wallaby)

Reviewed: https://review.opendev.org/c/openstack/manila/+/792663
Committed: https://opendev.org/openstack/manila/commit/7116f839e297a10ef99804d9b811d58cb3de5380
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 7116f839e297a10ef99804d9b811d58cb3de5380
Author: Goutham Pacha Ravi <email address hidden>
Date: Wed May 19 17:14:24 2021 -0700

    Fix with_entities usage in db queries

    In order to retrieve shares with filters
    pertaining to separate tables (shares, instances, metadata,
    etc), we need to perform joined loads of those
    respective database tables as necessary. After the
    join, the query cannot select entities due to
    Github issue #6253 - The following is an error when
    the pattern is used:

    sqlalchemy.exc.ArgumentError: Query has only expression-based entities - can't find property named "share_metadata".

    sqlalchemy 1.4 has a performance improvement that
    delays query processing [2] and disallows this
    pattern of usage; we can use a query.count() instead.

    In another instance, we can perform joins selectively
    only if asked for.

    [1] https://github.com/sqlalchemy/sqlalchemy/issues/625
    [2] https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#many-core-and-orm-statement-objects-now-perform-much-of-their-construction-and-validation-in-the-compile-phase

    Change-Id: I8aa196c171bbc224cec06f517ea22c4e91cbc06a
    Closes-Bug: #1926399
    Signed-off-by: Goutham Pacha Ravi <email address hidden>
    (cherry picked from commit 6ace54c2601770a0e06418ce5b98c756c86f6a51)

Revision history for this message
Balazs Gibizer (balazs-gibizer) wrote :
Changed in nova:
status: In Progress → Fix Released
tags: added: neutron-proactive-backport-potential
tags: removed: neutron-proactive-backport-potential
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.