Open Stack

> On Nov 19, 2014, at 12:59 PM, Boris Pavlovic <bpavlovic at mirantis.com> wrote:
> 
> Matthew, 
> 
> 
> LOL ORM on top of another ORM ....
> 
> https://img.neoseeker.com/screenshots/TW92aWVzL0RyYW1h/inception_image33.png <https://img.neoseeker.com/screenshots/TW92aWVzL0RyYW1h/inception_image33.png>

I know where you stand on this Boris, but I fail to see how this is a productive contribution to the discussion.  Leo Dicaprio isn’t going to solve our issue here and I look forward to iterating on what we have today.




> 
> 
> 
> Best regards,
> Boris Pavlovic 
> 
> On Wed, Nov 19, 2014 at 8:46 PM, Matthew Booth <mbooth at redhat.com <mailto: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 <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 <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 <mailto:OpenStack-dev at lists.openstack.org>
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev <http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev>
> 
> _______________________________________________
> 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/123abd8a/attachment.html>