Open Stack

We currently have a pattern in Nova where all database code lives in
db/sqla/api.py[1]. Database transactions are only ever created or used
in this module. This was an explicit design decision:
https://blueprints.launchpad.net/nova/+spec/db-session-cleanup .

However, it presents a problem when we consider NovaObjects, and
dependencies between them. For example, take Instance.save(). An
Instance has relationships with several other object types, one of which
is InstanceInfoCache. Consider the following code, which is amongst what
happens in spawn():

instance = Instance.get_by_uuid(uuid)
instance.vm_state = vm_states.ACTIVE
instance.info_cache.network_info = new_nw_info
instance.save()

instance.save() does (simplified):
  self.info_cache.save()
  self._db_save()

Both of these saves happen in separate db transactions. This has at
least 2 undesirable effects:

1. A failure can result in an inconsistent database. i.e. info_cache
having been persisted, but instance.vm_state not having been persisted.

2. Even in the absence of a failure, an external reader can see the new
info_cache but the old instance.

This is one example, but there are lots. We might convince ourselves
that the impact of this particular case is limited, but there will be
others where it isn't. Confidently assuring ourselves of a limited
impact also requires a large amount of context which not many
maintainers will have. New features continue to add to the problem,
including numa topology and pci requests.

I don't think we can reasonably remove the cascading save() above due to
the deliberate design of objects. Objects don't correspond directly to
their datamodels, so save() does more work than just calling out to the
DB. We need a way to allow cascading object saves to happen within a
single DB transaction. This will mean:

1. A change will be persisted either entirely or not at all in the event
of a failure.

2. A reader will see either the whole change or none of it.

We are not talking about crossing an RPC boundary. The single database
transaction only makes sense within the context of a single RPC call.
This will always be the case when NovaObject.save() cascades to other
object saves.

Note that we also have a separate problem, which is that the DB api's
internal use of transactions is wildly inconsistent. A single db api
call can result in multiple concurrent db transactions from the same
thread, and all the deadlocks that implies. This needs to be fixed, but
it doesn't require changing our current assumption that DB transactions
live only within the DB api.

Note that there is this recently approved oslo.db spec to make
transactions more manageable:

https://review.openstack.org/#/c/125181/11/specs/kilo/make-enginefacade-a-facade.rst,cm

Again, while this will be a significant benefit to the DB api, it will
not solve the problem of cascading object saves without allowing
transaction management at the level of NovaObject.save(): we need to
allow something to call a db api with an existing session, and we need
to allow something to pass an existing db transaction to NovaObject.save().

An obvious precursor to that is removing N309 from hacking, which
specifically tests for db apis which accept a session argument. We then
need to consider how NovaObject.save() should manage and propagate db
transactions.

I think the following pattern would solve it:

@remotable
def save():
    session = <insert magic here>
    try:
        r = self._save(session)
        session.commit() (or reader/writer magic from oslo.db)
        return r
    except Exception:
        session.rollback() (or reader/writer magic from oslo.db)
        raise

@definitelynotremotable
def _save(session):
    previous contents of save() move here
    session is explicitly passed to db api calls
    cascading saves call object._save(session)

Whether we wait for the oslo.db updates or not, we need something like
the above. We could implement this today by exposing
db.sqla.api.get_session().

Thoughts?

Matt

[1] At a slight tangent, this looks like an artifact of some premature
generalisation a few years ago. It seems unlikely that anybody is going
to rewrite the db api using an ORM other than sqlalchemy, so we should
probably ditch it and promote it to db/api.py.
-- 
Matthew Booth
Red Hat Engineering, Virtualisation Team

Phone: +442070094448 (UK)
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490