Investigate 'selectin' relation optimization with newer SQLAlchemy

Bug #2069061 reported by Miro Tomaska
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Triaged
Medium
Unassigned

Bug Description

We have attempted to optimize the number of sql queries neutron makes during testing with some "cheap wins" such as using `joined` instead of `subquery` relationship [1]. This helped with the number of queries but the actual optimization was never measured, it was just marked as "significant".
By doing this "cheap win" we have introduced regression in the development code for some cases where a large amount of tags is defined. This is documented in LP[2] and subsequently reverted in [3].

This LP was created to go back and explore the use of `selectin`[4] relation which should help with reducing the queries AND not cause regression with large deployments. The `selectin` option has been available since around SQLAlchemy 1.3 but given it was in its infancy back then, we should consider this with later sqlalchemy, e.i. 2.0+.

[1] https://review.opendev.org/c/openstack/neutron/+/891580
[2] https://launchpad.net/bugs/2068761
[3] https://review.opendev.org/c/openstack/neutron/+/921566
[4] https://docs.sqlalchemy.org/en/20/orm/queryguide/relationships.html#summary-of-relationship-loading-styles

Miro Tomaska (mtomaska)
Changed in neutron:
importance: Undecided → Medium
tags: added: db
Miro Tomaska (mtomaska)
description: updated
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (stable/zed)

Related fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/neutron/+/921780

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on neutron (stable/zed)

Change abandoned by "Miro Tomaska <email address hidden>" on branch: stable/zed
Review: https://review.opendev.org/c/openstack/neutron/+/921780
Reason: Note to self. Dont use gerrit revert option as it reverts to stable branch. Must revert on unmaintained branch

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (unmaintained/zed)

Related fix proposed to branch: unmaintained/zed
Review: https://review.opendev.org/c/openstack/neutron/+/921787

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (unmaintained/yoga)

Related fix proposed to branch: unmaintained/yoga
Review: https://review.opendev.org/c/openstack/neutron/+/921788

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (unmaintained/wallaby)

Related fix proposed to branch: unmaintained/wallaby
Review: https://review.opendev.org/c/openstack/neutron/+/921791

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (master)

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

commit bf123dfb385f85077e7e6c277da3789afa02f422
Author: Miro Tomaska <email address hidden>
Date: Fri Jun 7 19:14:00 2024 +0000

    Revert "Use HasStandardAttributes as parent class for Tags DB model"

    This reverts commit 85d3fff97e55ba85f72cda4365ad0441c10bd9f6.

    Reason for revert:
    The original change was made as a “cheap win” to optimize the number
    of queries the neutron server makes during testing. This did
    improve the number of queries made but introduced regression in
    real world deployments where some customers(through automation)
    would define hundreds of tags per port across a large deployment.
    I am proposing to revert this change in favor of the old “subquery”
    relation in order to fix this regression. In addition, I filed the
    Related-Bug #2069061 to investigate using `selectin` as the more
    appropriate long term solution.

    Change-Id: I83ec349e49e1f343da8996cab149d76443120873
    Closes-Bug: #2068761
    Related-Bug: #2069061

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (stable/2023.1)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/921569
Committed: https://opendev.org/openstack/neutron/commit/4f5c023279a0f9d0db8005848c8b356749ff4c8a
Submitter: "Zuul (22348)"
Branch: stable/2023.1

commit 4f5c023279a0f9d0db8005848c8b356749ff4c8a
Author: Miro Tomaska <email address hidden>
Date: Fri Jun 7 19:31:43 2024 +0000

    Revert "Use HasStandardAttributes as parent class for Tags DB model"

    This reverts commit b92fc1ad6c87c02537d790e63dd9c8a391682ae0.

    Reason for revert:
    The original change was made as a “cheap win” to optimize the number
    of queries the neutron server makes during testing. This did
    improve the number of queries made but introduced regression in
    real world deployments where some customers(through automation)
    would define hundreds of tags per port across a large deployment.
    I am proposing to revert this change in favor of the old “subquery”
    relation in order to fix this regression. In addition, I filed the
    Related-Bug #2069061 to investigate using `selectin` as the more
    appropriate long term solution.

    Change-Id: I437ab71f01483bb8adea96757a6d7dd6253df182
    Closes-bug: #2068761
    Related-Bug: #2069061

Changed in neutron:
status: New → Triaged
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (stable/2024.1)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/921567
Committed: https://opendev.org/openstack/neutron/commit/f7e9b2e5b31f9275036e644e13365ad78bb17b2b
Submitter: "Zuul (22348)"
Branch: stable/2024.1

commit f7e9b2e5b31f9275036e644e13365ad78bb17b2b
Author: Miro Tomaska <email address hidden>
Date: Fri Jun 7 19:14:00 2024 +0000

    Revert "Use HasStandardAttributes as parent class for Tags DB model"

    This reverts commit 85d3fff97e55ba85f72cda4365ad0441c10bd9f6.

    Reason for revert:
    The original change was made as a “cheap win” to optimize the number
    of queries the neutron server makes during testing. This did
    improve the number of queries made but introduced regression in
    real world deployments where some customers(through automation)
    would define hundreds of tags per port across a large deployment.
    I am proposing to revert this change in favor of the old “subquery”
    relation in order to fix this regression. In addition, I filed the
    Related-Bug #2069061 to investigate using `selectin` as the more
    appropriate long term solution.

    Change-Id: I83ec349e49e1f343da8996cab149d76443120873
    Closes-bug: #2068761
    Related-Bug: #2069061

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (stable/2023.2)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/921568
Committed: https://opendev.org/openstack/neutron/commit/2614e77bd161b619396b74c2670c13805468e156
Submitter: "Zuul (22348)"
Branch: stable/2023.2

commit 2614e77bd161b619396b74c2670c13805468e156
Author: Miro Tomaska <email address hidden>
Date: Fri Jun 7 19:14:00 2024 +0000

    Revert "Use HasStandardAttributes as parent class for Tags DB model"

    This reverts commit 85d3fff97e55ba85f72cda4365ad0441c10bd9f6.

    Reason for revert:
    The original change was made as a “cheap win” to optimize the number
    of queries the neutron server makes during testing. This did
    improve the number of queries made but introduced regression in
    real world deployments where some customers(through automation)
    would define hundreds of tags per port across a large deployment.
    I am proposing to revert this change in favor of the old “subquery”
    relation in order to fix this regression. In addition, I filed the
    Related-Bug #2069061 to investigate using `selectin` as the more
    appropriate long term solution.

    Change-Id: I83ec349e49e1f343da8996cab149d76443120873
    Closes-bug: #2068761
    Related-Bug: #2069061

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (unmaintained/yoga)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/921788
Committed: https://opendev.org/openstack/neutron/commit/f15f458861a9a2fbe763e2833c94f29d4b7b80ee
Submitter: "Zuul (22348)"
Branch: unmaintained/yoga

commit f15f458861a9a2fbe763e2833c94f29d4b7b80ee
Author: Miro Tomaska <email address hidden>
Date: Fri Jun 7 19:14:00 2024 +0000

    Revert "Use HasStandardAttributes as parent class for Tags DB model"

    This reverts commit 85d3fff97e55ba85f72cda4365ad0441c10bd9f6.

    Reason for revert:
    The original change was made as a “cheap win” to optimize the number
    of queries the neutron server makes during testing. This did
    improve the number of queries made but introduced regression in
    real world deployments where some customers(through automation)
    would define hundreds of tags per port across a large deployment.
    I am proposing to revert this change in favor of the old “subquery”
    relation in order to fix this regression. In addition, I filed the
    Related-Bug #2069061 to investigate using `selectin` as the more
    appropriate long term solution.

    Change-Id: I83ec349e49e1f343da8996cab149d76443120873
    Closes-Bug: #2068761
    Related-Bug: #2069061

tags: added: in-unmaintained-yoga
tags: added: in-unmaintained-wallaby
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (unmaintained/wallaby)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/921791
Committed: https://opendev.org/openstack/neutron/commit/26e9f35398db90f0cd07a7bb00ebb3453580e09b
Submitter: "Zuul (22348)"
Branch: unmaintained/wallaby

commit 26e9f35398db90f0cd07a7bb00ebb3453580e09b
Author: Miro Tomaska <email address hidden>
Date: Fri Jun 7 19:14:00 2024 +0000

    Revert "Use HasStandardAttributes as parent class for Tags DB model"

    This reverts commit 85d3fff97e55ba85f72cda4365ad0441c10bd9f6.

    Reason for revert:
    The original change was made as a “cheap win” to optimize the number
    of queries the neutron server makes during testing. This did
    improve the number of queries made but introduced regression in
    real world deployments where some customers(through automation)
    would define hundreds of tags per port across a large deployment.
    I am proposing to revert this change in favor of the old “subquery”
    relation in order to fix this regression. In addition, I filed the
    Related-Bug #2069061 to investigate using `selectin` as the more
    appropriate long term solution.

    Conflict:
            Wallaby version is not using `sync_backref=False` argument.
            So I just changed the lazy argument to subquery manually.

    Change-Id: I83ec349e49e1f343da8996cab149d76443120873
    Closes-Bug: #2068761
    Related-Bug: #2069061

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (unmaintained/zed)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/921787
Committed: https://opendev.org/openstack/neutron/commit/f60fce80903c6fd1a7a45df10b2d841ba52f55cd
Submitter: "Zuul (22348)"
Branch: unmaintained/zed

commit f60fce80903c6fd1a7a45df10b2d841ba52f55cd
Author: Miro Tomaska <email address hidden>
Date: Fri Jun 7 19:14:00 2024 +0000

    Revert "Use HasStandardAttributes as parent class for Tags DB model"

    This reverts commit 85d3fff97e55ba85f72cda4365ad0441c10bd9f6.

    Reason for revert:
    The original change was made as a “cheap win” to optimize the number
    of queries the neutron server makes during testing. This did
    improve the number of queries made but introduced regression in
    real world deployments where some customers(through automation)
    would define hundreds of tags per port across a large deployment.
    I am proposing to revert this change in favor of the old “subquery”
    relation in order to fix this regression. In addition, I filed the
    Related-Bug #2069061 to investigate using `selectin` as the more
    appropriate long term solution.

    Change-Id: I83ec349e49e1f343da8996cab149d76443120873
    Closes-Bug: #2068761
    Related-Bug: #2069061

tags: added: in-unmaintained-zed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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