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