> On Nov 19, 2014, at 12:59 PM, Boris Pavlovic <bpavlo...@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 <mbo...@redhat.com > <mailto:mbo...@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@lists.openstack.org <mailto:OpenStack-dev@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@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
_______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev