| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-20 | |||
| 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 | mdbooth | So this will continue to work for detach_volume. | |
| 18:17:12 | artom | Then we should be fine | |
| 18:17:23 | artom | Let me check whether I included this in the tempest tests | |
| 18:17:23 | mdbooth | But it wouldn't work for detach_ephemeral | |
| 18:17:29 | mdbooth | ...which doesn't exist. | |
| 18:17:30 | artom | ... | |
| 18:17:34 | artom | which haven't merged yet | |
| 18:17:40 | mdbooth | orly? | |
| 18:17:47 | mdbooth | I need to write tempest tests for my stuff. | |
| 18:17:56 | mdbooth | Can you ping me the reviews? | |
| 18:17:58 | artom | mdbooth, they'll merge in time for brexit | |
| 18:18:20 | artom | mdbooth, https://review.openstack.org/#/c/391947/ | |
| 18:18:44 | mdbooth | artom: You mean they're a complete trainwreck with inexplicable momentum marching inevitably towards their doom? | |
| 18:18:48 | artom | Yeah, they only test the attach case | |
| 18:18:51 | artom | I should add the detach case | |
| 18:19:03 | artom | mdbooth, isn't that all of openstack? ;) | |
| 18:19:19 | mdbooth | Ooh, harsh! :) | |
| 18:19:32 | artom | I'm an arse because I care? | |
| 18:21:14 | mdbooth | artom: Ok, looks like that needs a rebase. | |
| 18:21:47 | mdbooth | I'll take a look at that tomorrow, as I also need to write a test which boots with a tagged root/ephemeral/swap disk. | |
| 18:23:58 | openstackgerrit | rahul bardia proposed openstack/python-novaclient master: nova limits ERROR (Exception): Field names must be unique https://review.openstack.org/527922 | |
| 18:29:13 | artom | mdbooth, sure, thanks :) | |
| 18:29:22 | artom | I'll try to get to yours as well | |
| 18:29:24 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova-status check for ironic flavor migration https://review.openstack.org/527541 | |
| 18:45:34 | rybridges | Hey guys I have a quick question. Is there a way to list all instances for a particular user across all projects? | |
| 18:46:24 | rybridges | Is that what openstack server list --user <username> does? | |
| 18:49:21 | openstackgerrit | Chris Dent proposed openstack/nova master: Do not set allocation.id in AllocationList.create_all() https://review.openstack.org/529397 | |
| 18:49:41 | cdent | mriedem: there's ^ a first stab, I went for the simplest thing, for reasons listed in the commit message, gonna dine now. | |
| 18:52:17 | melwitt | rybridges: it appears so, it might require user uuid. and you might also need to pass --all-projects | |
| 18:53:21 | rybridges | ok | |
| 18:53:27 | rybridges | Thanks i will give it a shot! | |
| 18:55:38 | melwitt | let me know if it doesn't work. not that familiar with openstackclient but a glance at the code looks like it should do what you want | |
| 18:56:59 | rybridges | looks like this works: openstack server list --user rybridges --all-projects | |
| 18:57:09 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass RequestSpec to ConductorTaskAPI.build_instances https://review.openstack.org/515495 | |
| 18:57:10 | rybridges | but this does not: openstack server list --user rybridges | |
| 18:57:16 | rybridges | so ya you need --all-projects | |
| 18:58:27 | melwitt | rybridges: okay, thanks for confirming. without --all-projects it will default to filtering that user under the project making the request | |
| 18:58:47 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass RequestSpec to ConductorTaskAPI.build_instances https://review.openstack.org/515495 | |
| 18:59:30 | rybridges | that is correct melwitt | |
| 19:04:48 | edleafe | cdent: huh, that was way simpler than I thought it would be. | |
| 19:25:01 | cfriesen | maybe odd python style question, but why does nova code use super(ClassName, self).func() rather than super(type(self), self).func()? | |
| 19:25:43 | melwitt | I dunno. is the latter a more common pattern elsewhere? | |
| 19:26:58 | cfriesen | melwitt: well, in python 3 you can just use super(). the nice thing about the second one is that you don't need to update it if you subclass it and copy/paste a snippet of code. | |
| 19:29:03 | cfriesen | melwitt: heh...some additional googling answered my question. turns out in python2 it needs to be explicit to allow for multiple levels of inheritance. | |
| 19:29:39 | melwitt | ah, okay. now I know | |
| 19:39:22 | cdent | edleafe: I could have made it way more complex, but figured, what's the point? | |
| 19:45:59 | openstackgerrit | Chris Dent proposed openstack/nova master: Do not set allocation.id in AllocationList.create_all() https://review.openstack.org/529397 | |
| 19:46:33 | edleafe | cdent: I'm not disagreeing | |
| 19:47:56 | cdent | I know | |
| 19:50:56 | mriedem | edleafe: i think we're pretty close on https://review.openstack.org/#/c/511358/ but i don't know why you had to modify the FilterScheduler in there | |
| 19:52:50 | mriedem | i think you might be working around something in the claims code in the conductor build_instances method that i pointed out | |
| 19:53:07 | mriedem | also, i think claim_resources might be the wrong method to call from conductor, seems put_allocations is what we want instead | |
| 19:55:15 | cdent | I got things done today, but none of the things I mean to to do, so I'm giving up | |
| 19:55:38 | mriedem | that's most of my days | |
| 19:55:55 | cdent | true | |
| 19:56:14 | cdent | but today seemed especially bad. or maybe I just feel guilty for not doing the stuff I said I'd do for efried_cya_jan | |
| 19:56:16 | cdent | anyway | |
| 19:56:19 | cdent | goodnight | |
| 19:56:26 | SamYaple | /win/win 20 | |
| 20:03:05 | edleafe | mriedem: the filter scheduler change was because the functional tests turned up an issue: that the Selection objects in the alternates had no allocation_request in them. Since the Selection objects weren't used until this patch, it never showed up | |