Earlier  
Posted Nick Remark
#openstack-nova - 2017-12-06
18:52:42 jaypipes yeah, the update_with_match thing is the compare and swap
18:52:46 jaypipes regardless...
18:52:50 mdbooth https://review.openstack.org/#/c/242603/25/nova/objects/block_device.py
18:54:19 mdbooth jaypipes: Thing is, if you overthink the building blocks, it means you don't need to overthink everything else. Not so much, anyway.
18:54:44 mdbooth The caller doesn't need to be concerned with all these weird edge cases. It just works.
19:00:26 openstackgerrit Merged openstack/nova master: Fix doubling allocations on rebuild https://review.openstack.org/521662
19:01:51 zzzeek mdbooth / jaypipes the retry decorartor is wanting, IMO
19:02:05 zzzeek there was originally some idea to integrate one into enginefacade itself so that it knows how to nest
19:02:40 mdbooth zzzeek: Could you use a savepoint for this, btw?
19:03:10 zzzeek mdbooth: not if you're looking to respond to other transactions also making changes b.c. isolation may prevent you from seeing those changes
19:03:40 zzzeek for an UPDATE i think repeatable read would mean you can't see another transactions updates
19:05:31 mdbooth zzzeek: Anyway, my approach was to use an independent transaction context explicitly. I figured this would always be safe.
19:06:02 mdbooth Then I was convinced it wasn't worth writing slight weird code for an extremely obtuse edge case where failure was safe.
19:06:03 zzzeek mdbooth: i think you are correct assuming that record is not part of the external tranasction. pre-enginefacade nova was all independent transactions.
19:06:37 mdbooth But then I flipped again because I figured this is a read transaction which only needs to write in an edge case (migrating a legacy record).
19:07:10 mdbooth So having an independent transaction is useful to avoid forcing the enclosing transaction and all its potential parents to become writers.
19:08:06 mdbooth And that's where I am now.
19:10:38 zzzeek mdbooth: i think leave it and goad me into working up a better retry situation for enginefacade
19:11:08 mdbooth zzzeek: So all of the above is in the context of code we'd want to delete next cycle anyway, as it's only for a migration :)
19:11:48 mdbooth Well cycle +1, because the online migration runs it too.
19:11:48 zzzeek mdbooth: nova's magical migrations...sure
19:11:52 zzzeek mdbooth: sure
19:12:00 zzzeek mdbooth: this is not the most concerning thing to me :)
19:13:10 mdbooth I think it's fine, tbh. I also doubt that many of nova's uses of the retry wrapper are a problem today, because we don't nest many transactions.
19:13:35 mdbooth It would be a bigger problem if we made wider use of the enginefacade
19:14:29 mdbooth jaypipes: Thanks again
19:22:30 openstackgerrit Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766
19:33:20 cdent can someone besides mriedem put some eyes on https://review.openstack.org/#/c/523403/ is an auth config error that would be useful to backport. dansmith, jaypipes ?
19:33:55 efried cdent Patches are passing with your grenade fix - thanks for that.
19:34:06 cdent huzzah
19:34:30 mriedem cdent: i think melwitt enjoys auth config bug fixes
19:34:59 melwitt I totally do
19:35:02 cdent ah yes melwitt would be a fine choice
19:35:28 cdent apologies had not hear you around recently
19:35:35 cdent whereas dansmith and jaypipes …
19:35:57 efried cdent Is the kwarg really called oslo_config_config?
19:36:19 cdent yeah, isn’t that awesome, there are a few different olso_config_*
19:36:54 efried whee
19:37:42 cdent I think it is to try to be explicit about the fact that it can also accept keyword arguments which are configuration keys and values
19:38:05 cdent a sort of namespacing
19:41:27 sean-k-mooney2 cdent: we were going to do something similar with networking-ovs-dpdk devstack setting then reallised that would haver ment setting OVS_DPDK_OVS_REPO AND OVS_DPDK_DPDK_REPO so decided not to prepend everyting with OVS_DPDK
19:42:08 cdent oh my
19:42:52 sean-k-mooney2 we still have pretty verbose setting without that https://github.com/openstack/networking-ovs-dpdk/blob/master/devstack/settings
19:42:53 efried cdent Left a comment on that patch. Is there a reason we would need to keep that arg?
19:43:49 cdent efried: because we don’t know who might have use the deploy() method in the past
19:44:01 cdent that’s how you build a custom wsgi application, if you’re so inclined
19:44:07 efried cdent Is it somehow public?
19:44:24 cdent it is public by virtue of being python
19:44:47 efried What I mean is, is it used by some known external entity that we care about?
19:44:52 cdent and in the early days of placement was the entry point to the wsgi application stack until sean added the stuff in wsgi.py
19:44:57 cdent the point is that we don’t know
19:45:09 efried But we care?
19:45:13 cdent yes, of course
19:45:18 efried Oh, not of course.
19:45:33 cdent the whole point of a wsgi application is to be able to integrate it in whatever way you want
19:45:36 efried Nova changes "public APIs" like ComputeDriver all the time without worrying about external consumption.
19:45:47 sean-k-mooney2 was it ever the entrypoint in a version of the placement api that shipped in a release
19:46:07 efried And when I've asked about that before, it was basically explained away to "we don't care".
19:46:15 efried So in this case, why do we care?
19:46:16 cdent sean-k-mooney2: a) I _think_ so but am not certain, b) it has always been my intent that deploy be available as a public interface
19:46:28 cdent because placement is not nova?
19:46:46 cdent it has a _much_ different attitude towards building the wsgi application than the nova-api ever did
19:47:13 sean-k-mooney2 cdent: well if that was the intent then we should treat it as public and document that fact.
19:48:13 cdent I think I probably made the mistake of assuming that was somehow obvious, by virtue of its position in the wsgi stack, but I think I may be one of the few people around here who think of the wsgi stack in that fashion
19:48:23 cdent we can certainly document it as such
19:49:09 cdent efried: would your preference be that I add that documentation to that patch now, or is it cool to worry about it later?
19:49:32 efried Later is fine.
19:50:50 efried cdent Changed sides.
19:51:25 cdent thanks, I’ll do the documenting likely tomorrow, I think I’m out of code mode for today
19:51:36 cdent also, all my promises to myself to not work late tonight seem to be for naught
19:52:21 sean-k-mooney2 cdent: your based in the uk right?
19:52:24 cdent yeah
19:52:33 cdent and I often work this late, but last night I worked past midnight
19:52:40 cdent so was going to give myself a break
19:52:50 cdent but everybody here was just too interesting today
19:53:14 sean-k-mooney2 i have a daily reminder to head home at 8 so im normally here till around now too.
19:56:00 cdent as it is 5 to 8 I think I’ll go, have a good night every one
20:09:16 melwitt mriedem: in case you didn't see, I had posted a follow up for the old quotas code removal that we talked about some days ago https://review.openstack.org/#/c/524234
20:12:29 melwitt although I think I just realized an issue with it and commented
20:12:43 mriedem lgtm
20:13:19 mriedem oh right
20:13:30 mriedem gawd
20:14:21 melwitt yeah. guh
20:15:33 mriedem https://review.openstack.org/#/c/400384/
20:15:58 melwitt I need to add some kind of equivalent test to the API tests somewhere. the api samples test cover the presence of the 'reserved' key but not the expected value if no-op quotas
20:16:13 mriedem well,
20:16:27 mriedem easiest is probably an api samples test with the noop quota driver and it's own samples
20:23:56 melwitt lemme see if I can do that
20:25:33 openstackgerrit Hongbin Lu proposed openstack/nova master: Add support for listing hosts in cellv2 https://review.openstack.org/524755
20:41:21 openstackgerrit Matt Riedemann proposed openstack/nova master: Add new style volume attachment support to block_device.py https://review.openstack.org/525787
20:41:22 openstackgerrit Matt Riedemann proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285
20:41:22 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: libvirt: Allow multiple volume attachments https://review.openstack.org/267587
20:41:23 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Allow multi-attach in compute api https://review.openstack.org/271047
20:41:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Avoid stashed connector lookup for new style detach https://review.openstack.org/526182
20:46:51 efried jaypipes I seem to have flubbed https://review.openstack.org/#/c/521605/ which was supposed to be two separate commits (one to add agg stuff to ProviderTree, one to add traits stuff).
20:47:05 efried jaypipes I was about to split it up, but thought I should ask you whether it's worth bothering
20:47:53 efried jaypipes i.e. should I just do both in one change set?
21:03:46 mriedem ildikov: ok, final comments in https://review.openstack.org/#/c/330285/173
21:03:54 mriedem ildikov: easy things to address and then i'm ready to go on that
21:04:03 ildikov mriedem: ok

Earlier   Later