<div dir="ltr"><div>Matthew, <br></div><div><br></div><div><br></div>LOL ORM on top of another ORM ....<div><br></div><div><a href="https://img.neoseeker.com/screenshots/TW92aWVzL0RyYW1h/inception_image33.png">https://img.neoseeker.com/screenshots/TW92aWVzL0RyYW1h/inception_image33.png</a><br></div><div><br></div><div><br></div><div><br></div><div>Best regards,</div><div>Boris Pavlovic </div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 19, 2014 at 8:46 PM, Matthew Booth <span dir="ltr"><<a href="mailto:mbooth@redhat.com" target="_blank">mbooth@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">We currently have a pattern in Nova where all database code lives in<br>
db/sqla/api.py[1]. Database transactions are only ever created or used<br>
in this module. This was an explicit design decision:<br>
<a href="https://blueprints.launchpad.net/nova/+spec/db-session-cleanup" target="_blank">https://blueprints.launchpad.net/nova/+spec/db-session-cleanup</a> .<br>
<br>
However, it presents a problem when we consider NovaObjects, and<br>
dependencies between them. For example, take Instance.save(). An<br>
Instance has relationships with several other object types, one of which<br>
is InstanceInfoCache. Consider the following code, which is amongst what<br>
happens in spawn():<br>
<br>
instance = Instance.get_by_uuid(uuid)<br>
instance.vm_state = vm_states.ACTIVE<br>
instance.info_cache.network_info = new_nw_info<br>
instance.save()<br>
<br>
instance.save() does (simplified):<br>
self.info_cache.save()<br>
self._db_save()<br>
<br>
Both of these saves happen in separate db transactions. This has at<br>
least 2 undesirable effects:<br>
<br>
1. A failure can result in an inconsistent database. i.e. info_cache<br>
having been persisted, but instance.vm_state not having been persisted.<br>
<br>
2. Even in the absence of a failure, an external reader can see the new<br>
info_cache but the old instance.<br>
<br>
This is one example, but there are lots. We might convince ourselves<br>
that the impact of this particular case is limited, but there will be<br>
others where it isn't. Confidently assuring ourselves of a limited<br>
impact also requires a large amount of context which not many<br>
maintainers will have. New features continue to add to the problem,<br>
including numa topology and pci requests.<br>
<br>
I don't think we can reasonably remove the cascading save() above due to<br>
the deliberate design of objects. Objects don't correspond directly to<br>
their datamodels, so save() does more work than just calling out to the<br>
DB. We need a way to allow cascading object saves to happen within a<br>
single DB transaction. This will mean:<br>
<br>
1. A change will be persisted either entirely or not at all in the event<br>
of a failure.<br>
<br>
2. A reader will see either the whole change or none of it.<br>
<br>
We are not talking about crossing an RPC boundary. The single database<br>
transaction only makes sense within the context of a single RPC call.<br>
This will always be the case when NovaObject.save() cascades to other<br>
object saves.<br>
<br>
Note that we also have a separate problem, which is that the DB api's<br>
internal use of transactions is wildly inconsistent. A single db api<br>
call can result in multiple concurrent db transactions from the same<br>
thread, and all the deadlocks that implies. This needs to be fixed, but<br>
it doesn't require changing our current assumption that DB transactions<br>
live only within the DB api.<br>
<br>
Note that there is this recently approved oslo.db spec to make<br>
transactions more manageable:<br>
<br>
<a href="https://review.openstack.org/#/c/125181/11/specs/kilo/make-enginefacade-a-facade.rst,cm" target="_blank">https://review.openstack.org/#/c/125181/11/specs/kilo/make-enginefacade-a-facade.rst,cm</a><br>
<br>
Again, while this will be a significant benefit to the DB api, it will<br>
not solve the problem of cascading object saves without allowing<br>
transaction management at the level of NovaObject.save(): we need to<br>
allow something to call a db api with an existing session, and we need<br>
to allow something to pass an existing db transaction to NovaObject.save().<br>
<br>
An obvious precursor to that is removing N309 from hacking, which<br>
specifically tests for db apis which accept a session argument. We then<br>
need to consider how NovaObject.save() should manage and propagate db<br>
transactions.<br>
<br>
I think the following pattern would solve it:<br>
<br>
@remotable<br>
def save():<br>
session = <insert magic here><br>
try:<br>
r = self._save(session)<br>
session.commit() (or reader/writer magic from oslo.db)<br>
return r<br>
except Exception:<br>
session.rollback() (or reader/writer magic from oslo.db)<br>
raise<br>
<br>
@definitelynotremotable<br>
def _save(session):<br>
previous contents of save() move here<br>
session is explicitly passed to db api calls<br>
cascading saves call object._save(session)<br>
<br>
Whether we wait for the oslo.db updates or not, we need something like<br>
the above. We could implement this today by exposing<br>
db.sqla.api.get_session().<br>
<br>
Thoughts?<br>
<br>
Matt<br>
<br>
[1] At a slight tangent, this looks like an artifact of some premature<br>
generalisation a few years ago. It seems unlikely that anybody is going<br>
to rewrite the db api using an ORM other than sqlalchemy, so we should<br>
probably ditch it and promote it to db/api.py.<br>
<span class="HOEnZb"><font color="#888888">--<br>
Matthew Booth<br>
Red Hat Engineering, Virtualisation Team<br>
<br>
Phone: +442070094448 (UK)<br>
GPG ID: D33C3490<br>
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490<br>
<br>
_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</font></span></blockquote></div><br></div>