On 12/11/14 19:39, Mike Bayer wrote: > >> On Nov 12, 2014, at 12:45 PM, Dan Smith <d...@danplanet.com> wrote: >> >>> I personally favour having consistent behaviour across the board. >>> How about updating them all to auto-refresh by default for >>> consistency, but adding an additional option to save() to disable >>> it for particular calls? >> >> I think these should be two patches: one to make them all >> auto-refresh, and another to make it conditional. That serves the >> purpose of (a) bisecting a regression to one or the other, and (b) >> we can bikeshed on the interface and appropriateness of the >> don't-refresh flag :) >> >>> I also suggest a tactical fix to any object which fetches itself >>> twice on update (e.g. Aggregate). >> >> I don't see that being anything other than an obvious win, unless >> there is some obscure reason for it. But yeah, seems like a good >> thing to do. > > lets keep in mind my everyone-likes-it-so-far proposal for reader() > and writer(): https://review.openstack.org/#/c/125181/ (this is > where it’s going to go as nobody has -1’ed it, so in absence of any > “no way!” votes I have to assume this is what we’re going with).
FWIW, it got my +1, too. Looks great. > in this system, the span of session use is implicit within the > context and/or decorator, and when writer() is specified, a commit() > can be implicit as well. IMHO there should be no “.save()” at all, > at least as far as database writing is concerned. SQLAlchemy > doesn’t need boilerplate like that - just let the ORM work normally: > > @sql.writer def some_other_api_method(context): someobject = > context.session.query(SomeObject)….one() > someobject.change_some_state(<stuff>) > > # done! > > if you want an explicit refresh, then just do so: > > @sql.writer def some_other_api_method(context): someobject = > context.session.query(SomeObject)….one() > someobject.change_some_state(<stuff>) > > context.session.flush() context.session.refresh(someobject) # do > something with someobject Unfortunately this model doesn't apply to Nova objects, which are persisted remotely. Unless I've missed something, SQLA doesn't run on Nova Compute at all. Instead, when Nova Compute calls object.save() this results in an RPC call to Nova Conductor, which persists the object in the DB using SQLA. Compute wouldn't be able to use common DB transactions without some hairy lifecycle management in Conductor, so Compute apis need to be explicitly aware of this. However, it absolutely makes sense for a single Conductor api call to use a single transaction. > however, seeing as this is all one API method the only reason you’d > want to refresh() is that you think something has happened between > that flush() and the refresh() that would actually show up, I can’t > imagine what that would be looking for, unless maybe some large > amount of operations took up a lot of time between the flush() and > the refresh(). Given the above constraints, the problem I'm actually trying to solve is when another process modifies an object underneath us between multiple, remote transactions. This is one of the motivations for compare-and-swap over row locking on read. Another is that the length of some API calls makes holding a row lock for that long undesirable. Matt -- 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