oslo.db is not able to cope with multiple engines

Bug #1263908 reported by Julien Danjou
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Ceilometer
Fix Released
High
Julien Danjou
oslo-incubator
Fix Released
High
Roman Podoliaka

Bug Description

oslo.db has a very poor behaviour currently as it uses a lot of global variable.

First it uses oslo.config.cfg.CONF in a lot of places (especially session) making it difficult to cooperate nicely with unit tests.

Then, it uses a global variable called _ENGINE that represents "the" engine used. However, if you want to use several engines at the same time, everything explodes and nothing works at all, as the library gets confused.

Tags: db
Revision history for this message
Julien Danjou (jdanjou) wrote :

I'm adding Ceilometer as affected, because that makes impossible for it to run unit tests against multiple SQL backend at the same time, nor to use multiple backends at the same time.

Changed in oslo:
assignee: nobody → Julien Danjou (jdanjou)
Changed in ceilometer:
status: New → Triaged
importance: Undecided → High
assignee: nobody → Julien Danjou (jdanjou)
Julien Danjou (jdanjou)
tags: added: db
Revision history for this message
Doug Hellmann (doug-hellmann) wrote :

This bug will probably result in an API change, so it will block releasing oslo.db as a library.

Changed in oslo:
importance: Undecided → High
status: New → Triaged
Mehdi Abaakouk (sileht)
Changed in oslo:
assignee: Julien Danjou (jdanjou) → Mehdi Abaakouk (sileht)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to oslo-incubator (master)

Fix proposed to branch: master
Review: https://review.openstack.org/68684

Changed in oslo:
assignee: Mehdi Abaakouk (sileht) → Roman Podoliaka (rpodolyaka)
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.openstack.org/68693

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to oslo-incubator (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/68893

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Related fix proposed to branch: master
Review: https://review.openstack.org/69123

Changed in oslo:
assignee: Roman Podoliaka (rpodolyaka) → Victor Sergeyev (vsergeyev)
Changed in oslo:
assignee: Victor Sergeyev (vsergeyev) → Alexei Kornienko (alexei-kornienko)
Changed in oslo:
assignee: Alexei Kornienko (alexei-kornienko) → Victor Sergeyev (vsergeyev)
Changed in oslo:
assignee: Victor Sergeyev (vsergeyev) → Roman Podoliaka (rpodolyaka)
Changed in oslo:
assignee: Roman Podoliaka (rpodolyaka) → Victor Sergeyev (vsergeyev)
Changed in oslo:
assignee: Victor Sergeyev (vsergeyev) → Roman Podoliaka (rpodolyaka)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to oslo-incubator (master)

Reviewed: https://review.openstack.org/68684
Committed: https://git.openstack.org/cgit/openstack/oslo-incubator/commit/?id=ce69e7f8f65b175343d7035b3f084c91fd29fbf5
Submitter: Jenkins
Branch: master

commit ce69e7f8f65b175343d7035b3f084c91fd29fbf5
Author: Roman Podoliaka <email address hidden>
Date: Thu Jan 23 17:45:50 2014 +0200

    Don't store engine instances in oslo.db

    oslo.db is meant to be a collection of helper utilities for
    SQLAlchemy. In order to behave like a 'good' library, we must
    stop using global state.

    This patch ensures we don't store any engine instances globally in
    oslo.db. It's up to end applications to decide how to cope with
    engines, not oslo.db.

    Partial-Bug: #1263908

    Co-authored-by: Victor Sergeyev <email address hidden>

    Change-Id: I330467ec1317e1a13ff9f0237e2d8900d718e379

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/68693
Committed: https://git.openstack.org/cgit/openstack/oslo-incubator/commit/?id=630d3959b9d001ca18bd2ed1cf757f2eb44a336f
Submitter: Jenkins
Branch: master

commit 630d3959b9d001ca18bd2ed1cf757f2eb44a336f
Author: Roman Podoliaka <email address hidden>
Date: Thu Jan 23 18:42:56 2014 +0200

    Don't use cfg.CONF in oslo.db

    Using of a global config object is something we want to avoid when
    making libraries. Parameters must be passed as function/class methods
    arguments instead.

    In order not to duplicate options declaration in each project, they
    have been put into separate options module (which should not be a
    part of oslo.db when it's released; we'll probably want to find a
    place for it within oslo-incubator).

    Partial-Bug: #1263908

    Co-authored-by: Victor Sergeyev <email address hidden>

    Change-Id: Ib5a4e31b4e78e2b4cb807a8c88b8072f4207eb22

Revision history for this message
Viktor Serhieiev (vsergeyev) wrote :

Two following patches should fix this bug

Changed in oslo:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in oslo:
milestone: none → icehouse-rc1
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in oslo:
milestone: icehouse-rc1 → 2014.1
Revision history for this message
gordon chung (chungg) wrote :

are we indirectly fixed in ceilometer as well?

Revision history for this message
Viktor Serhieiev (vsergeyev) wrote :

Cielometer uses oslo.db for a long tine, so this bug should not affect it anymore

Changed in ceilometer:
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

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