Open Stack

On 12/11/14 19:39, Mike Bayer wrote:
> 
>> On Nov 12, 2014, at 12:45 PM, Dan Smith <dms at 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