[openstack-dev] [oslo.db][nova] NovaObject.save() needs its own DB transaction
Boris Pavlovic
bpavlovic at mirantis.com
Wed Nov 19 17:59:27 UTC 2014
Matthew,
LOL ORM on top of another ORM ....
https://img.neoseeker.com/screenshots/TW92aWVzL0RyYW1h/inception_image33.png
Best regards,
Boris Pavlovic
On Wed, Nov 19, 2014 at 8:46 PM, Matthew Booth <mbooth at redhat.com> wrote:
> 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
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141119/9c7a3cd9/attachment.html>
More information about the OpenStack-dev
mailing list