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@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
