Posted	Nick	Remark
#openstack-nova - 2017-12-20
17:07:34	openstackgerrit	Merged openstack/nova master: [placement] Separate API schemas (allocation_candidate) https://review.openstack.org/528635
17:10:42	cdent	mriedem: that's rather novel
17:14:24	mriedem	yeah 2 hits in 7 days
17:16:27	cdent	the code is claiming that that error should only happen when the incoming allocation objects already have an 'id' field
17:18:37	cdent	which will take some digging to figure out how is even possible
17:19:57	openstackgerrit	Stephen Finucane proposed openstack/nova master: Move aggregates from report client to ProviderTree https://review.openstack.org/521685
17:19:58	openstackgerrit	Stephen Finucane proposed openstack/nova master: Track provider traits in report client https://review.openstack.org/521686
17:19:58	openstackgerrit	Stephen Finucane proposed openstack/nova master: Track associated sharing RPs in report client https://review.openstack.org/526539
17:19:58	openstackgerrit	Stephen Finucane proposed openstack/nova master: Raise on API errors getting aggregates/traits https://review.openstack.org/526540
17:19:59	openstackgerrit	Stephen Finucane proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756
17:19:59	openstackgerrit	Stephen Finucane proposed openstack/nova master: Track tree-associated providers in report client https://review.openstack.org/526541
17:20:00	openstackgerrit	Stephen Finucane proposed openstack/nova master: WIP: Add nested resources to server moving tests https://review.openstack.org/527728
17:20:54	stephenfin	jaypipes: Think there might be misunderstanding going on here https://review.openstack.org/#/c/527472/
17:21:19	stephenfin	Not saying I don't do overly clever things. Just that this isn't one of them ;)
17:21:33	openstackgerrit	Stephen Finucane proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098
17:21:53	openstackgerrit	Stephen Finucane proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187
17:22:04	openstackgerrit	Stephen Finucane proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246
17:26:46	cdent	mriedem: I think I figured out what's up, I left a comment on the bug, assigned myself, and will try a fix
17:27:43	mriedem	cdent: ah yeah didn't see the retry decorator
17:27:44	mriedem	nice catch
17:28:10	cdent	I can't decide what to shake it at, my my tiny fist is shaking
17:28:16	cdent	s/my my/but my/
17:28:56	mriedem	yeah not sure how to fix that
17:29:30	mdbooth	Sorry in advance, had a merge conflict half way up :/
17:29:58	openstackgerrit	Matthew Booth proposed openstack/nova master: Add uuid column to BlockDeviceMapping https://review.openstack.org/242602
17:29:58	openstackgerrit	Matthew Booth proposed openstack/nova master: Make BlockDeviceMapping object support uuid https://review.openstack.org/242603
17:29:59	openstackgerrit	Matthew Booth proposed openstack/nova master: Add an online migration for BDM.uuid https://review.openstack.org/525599
17:29:59	openstackgerrit	Matthew Booth proposed openstack/nova master: DriverBlockDevice: make subclasses inherit _proxy_as_attr https://review.openstack.org/524167
17:30:00	openstackgerrit	Matthew Booth proposed openstack/nova master: Expose BDM uuid to drivers https://review.openstack.org/529037
17:30:00	openstackgerrit	Matthew Booth proposed openstack/nova master: Give volume DriverBlockDevice classes a common prefix https://review.openstack.org/526346
17:30:01	openstackgerrit	Matthew Booth proposed openstack/nova master: Add DriverLocalImageBlockDevice https://review.openstack.org/526347
17:30:01	openstackgerrit	Matthew Booth proposed openstack/nova master: Rename block_device_info_get_root https://review.openstack.org/529028
17:30:02	openstackgerrit	Matthew Booth proposed openstack/nova master: Add local_root to block_device_info https://review.openstack.org/529029
17:30:02	openstackgerrit	Matthew Booth proposed openstack/nova master: Expose driver_block_device fields as attributes https://review.openstack.org/528362
17:30:03	openstackgerrit	Matthew Booth proposed openstack/nova master: Pass DriverBlockDevice to driver.attach_volume https://review.openstack.org/528363
17:30:03	openstackgerrit	Matthew Booth proposed openstack/nova master: Use real block_device_info data in libvirt tests https://review.openstack.org/527916
17:30:04	openstackgerrit	Matthew Booth proposed openstack/nova master: Fix libvirt volume tests passing invalid disk_info https://review.openstack.org/529328
17:30:04	openstackgerrit	Matthew Booth proposed openstack/nova master: Pass disk_info dict to libvirt_info https://review.openstack.org/529329
17:30:05	openstackgerrit	Matthew Booth proposed openstack/nova master: Local disk serial numbers for the libvirt driver https://review.openstack.org/529380
17:36:46	cdent	jaypipes: can you recall why/if it is important that AllocationList.create_all() is side effecty on the objects that it passes in? https://github.com/openstack/nova/blob/master/nova/tests/unit/objects/test_resource_provider.py#L355-L357 and https://github.com/openstack/nova/blob/609ddc2244bf9ba84d66cced52e7d4e2987ec669/nova/objects/resource_provider.py#L2061-L2086 and https://bugs.launchpad.net/nova/+bug/1739453
17:36:47	openstack	Launchpad bug 1739453 in OpenStack Compute (nova) "MigrationsAdminTest fails with NoValidHost because resource claim swap in placement fails with 500" [High,Confirmed] - Assigned to Chris Dent (cdent)
17:41:15	melwitt	mriedem: FYI the consoles series is ready to go, complete with devstack patch sandwich around it for the tempest vnc tests. stephenfin has gone through it already https://review.openstack.org/#/q/topic:bp/convert-consoles-to-objects+(status:open+OR+status:merged)
17:41:57	mriedem	devstack sandwich worked?
17:42:06	melwitt	yes. I owe you a cream soda
17:42:11	mriedem	awesome
17:43:17	edleafe	cdent: if the allocation had an id, then it already exists, no?
17:44:07	cdent	no, it's being assigned the .id, but then db deadlock happens and the transaction is reverted and started again, but the list of allocs now contains that alloc with an id
17:44:25	mdbooth	artom: Do I recall you being involved in device tagging?
17:44:56	edleafe	cdent: ah, so it should change the object, but it's missing the object revert?
17:46:17	cdent	edleafe: that's one way to look at it. The other way is that we should be side effecty on on method parameters that are list or dict when we have an option to re-enter. So I'm trying to figure out how to change that in the best way
17:46:29	cdent	dammit, should _not_ be side effecty
17:47:15	mriedem	cdent: right if you copy the allocs list, then retry, you'll likely then hit a duplicate entry error
17:47:23	edleafe	ok, I see what you're saying. It's modifying a mutable param
17:48:32	cdent	i need to poke at what the db retry handler is actually doing to be sure
17:50:32	mriedem	maybe we need to move the insert portion to another method and that method has the retry on deadlock handler
17:50:47	mriedem	apart from the _set_allocations code that's checking the object id
17:51:03	mriedem	but,
17:51:19	mriedem	we could still then hit duplicate entry errors i'd think
17:51:26	mriedem	any time you're retrying to insert over a list...
17:52:18	mriedem	so loop over the objects, then insert separately per object in another method with the deadlock retry on that method? would that work?
17:54:21	openstackgerrit	Matt Riedemann proposed openstack/nova stable/pike: Use instance.project_id when creating request specs for old instances https://review.openstack.org/529384
17:54:22	openstackgerrit	Matt Riedemann proposed openstack/nova stable/pike: Workaround missing RequestSpec.project_id when moving an instance https://review.openstack.org/529385
17:55:11	cdent	mriedem: unclear. still trying to get all the pieces to line up. one thing that's breaking my brain is that the retry wrapper is not explicitly saying anything about the current transaction. will keep looking
17:56:58	cdent	mriedem: i think you're right though, extracting a method for the individual insert seems right
17:57:49	mriedem	seems easiest anyway
18:03:45	openstackgerrit	Matt Riedemann proposed openstack/nova stable/ocata: Use instance.project_id when creating request specs for old instances https://review.openstack.org/529387
18:08:21	openstackgerrit	Matt Riedemann proposed openstack/nova stable/newton: Use instance.project_id when creating request specs for old instances https://review.openstack.org/529389
18:08:48	cdent	mriedem: hmmm, jay's comment from when he added the retry is potentially derailing, he's got a diferent theory of what could be causing the issues: https://github.com/openstack/nova/commit/e001b0538a9d413f864
18:10:06	mriedem	that likely makes more sense, the inventory thing
18:10:43	mriedem	but in that case, the deadlock isn't on the insert, it's on the generation update yeah? https://github.com/openstack/nova/commit/e001b0538a9d413f864#diff-a040eef16b56cc7862fb2a334c757deaR1855
18:12:02	cdent	right, which I'm not sure we have any way of knowing
18:12:59	cdent	the easiest fix is just simply not doing alloc.id = lastrowid
18:13:05	mriedem	http://logs.openstack.org/84/529184/2/check/legacy-tempest-dsvm-py35/888d647/logs/screen-placement-api.txt.gz#_Dec_20_15_15_19_616413
18:13:13	mriedem	Dec 20 15:15:19.616413 ubuntu-xenial-citycloud-lon1-0001533915 devstack@placement-api.service[15195]: DEBUG oslo_db.api [None req-d117a878-c8e0-4c3b-a073-848d73575036 service placement] Performing DB retry for function nova.objects.resource_provider.AllocationList._set_allocations {{(pid=15198) wrapper /usr/local/lib/python3.5/dist-packages/oslo_db/api.py:149}}
18:13:20	mriedem	at least it gets logged :)
18:13:49	cdent	oh, hmmm
18:13:59	cdent	that's saying it is on the insert
18:14:17	mriedem	where does it say that?
18:14:23	cdent	previous line?
18:14:30	mdbooth	artom: Hey, I just added a patch which makes it meaningful to tag a local disk
18:14:43	mdbooth	As local disks can now have serial numbers
18:15:11	artom	mdbooth, https://review.openstack.org/#/c/529380/?
18:15:18	mdbooth	The way disk tagging works, though, is that when deleting disk metadata it 'matches' based on device.serial, which it assumes to be volume_id
18:15:22	mdbooth	Although..........
18:15:32	mdbooth	Wait a sec, I might be trying to solve a problem which doesn't exist
18:15:40	mdbooth	This is only relevant in detach_volume
18:15:46	mdbooth	And as you can't detach a local disk....
18:15:49	mdbooth	There's no problem here
18:16:26	mdbooth	artom: That's the one, yeah.
18:16:36	mdbooth	artom: Ok, stand down. I don't think this is a thing.
18:16:43	mdbooth	However, if you want to review that series ;)
18:16:53	artom	Also, if I read your patch correctly, it still uses volume UUID as the serial for volumes
18:16:56	artom	So nothing changes there
18:17:05	mdbooth	artom: Right, that's correct.
18:17:12	artom	Then we should be fine
18:17:12	mdbooth	So this will continue to work for detach_volume.
18:17:23	mdbooth	But it wouldn't work for detach_ephemeral
18:17:23	artom	Let me check whether I included this in the tempest tests
18:17:29	mdbooth	...which doesn't exist.
18:17:30	artom	...

Earlier   Later