| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-30 | |||
| 15:21:27 | mdbooth | I was actually trying to work out where we set the boot index of a root disk to 0 if it wasn't done explicitly in the api call | |
| 15:21:30 | mdbooth | still haven't found it | |
| 15:21:50 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add instance action record for shelve_offload instances https://review.openstack.org/523653 | |
| 15:23:31 | openstackgerrit | Merged openstack/nova stable/pike: Set regex flag on ostestr command for osprofiler tests https://review.openstack.org/510226 | |
| 15:23:45 | mriedem | melwitt: that didn't seem to fix it, still can't find host2 in cell2 | |
| 15:24:31 | tetsuro | Hi all, I'd like you to review https://review.openstack.org/#/c/465160/ | |
| 15:26:11 | tetsuro | Currently, VM with NUMA goes into a silent error if the virt_type doesn't support it. | |
| 15:26:21 | tetsuro | So, this patch is to raise an error explicitly if NUMA Topology not supported, | |
| 15:49:06 | mriedem | 2017-11-30 10:47:07,834 ERROR [nova.tests.functional.test_servers] Found host host2 in other cell cell1 | |
| 15:49:06 | mriedem | 2017-11-30 10:47:07,831 ERROR [nova.tests.functional.test_servers] Failed to find host host2 in cell cell2 | |
| 15:49:18 | mriedem | yeah, so for whatever reason, even if i create host2 in cell2, it only ever comes back from cell1 | |
| 15:50:29 | dansmith | mriedem: I commented on that migration sorting deal | |
| 15:51:13 | dansmith | I think I deserve a cookie and a nap now | |
| 15:52:30 | mriedem | heh, thanks, looking | |
| 15:52:34 | mriedem | i did lose some sleep over this last night | |
| 15:52:46 | mriedem | this and that god forsaken stray cat | |
| 15:53:31 | dansmith | hehe | |
| 16:00:12 | mriedem | dansmith: replied, | |
| 16:00:23 | mriedem | this doesn't deal with the global/local marker issue either, | |
| 16:00:24 | efried | mriedem Thanks for the pointer. After adding in the suggested code and finding & fixing the template files, I got the tests passing. <== cdent jaypipes sdague FYI | |
| 16:00:31 | mriedem | but i'm admittedly a bit lost on the local marker logic and how that all works | |
| 16:00:47 | mriedem | dansmith: i mentioned the cell-encoded marker idea too, but not entirely sure how we can use that to fix the local marker thing | |
| 16:01:05 | mriedem | if you look at PS21, he had a near exact copy of the instance list stuff with the global/local marker | |
| 16:01:48 | dansmith | mriedem: right, which is what I called out with him using the marker uuid in all the cells | |
| 16:02:05 | ildikov | mriedem: meeting on #openstack-meeting-cp if you have a few minutes | |
| 16:03:34 | mriedem | ildikov: you must have missed my o/ | |
| 16:03:54 | ildikov | mriedem: you must've done it while I was typing here :) | |
| 16:04:20 | dansmith | mriedem: the cell encoding in the marker could help in a few cases: | |
| 16:04:33 | dansmith | mriedem: if we encode the cell, then we can avoid looking in all cells for the marker uuid as the first step | |
| 16:04:47 | mriedem | yup, i at least understand that part so far | |
| 16:05:36 | dansmith | mriedem: if we encoded the cell *and* the value we're sorting on of the marker, then we could find the marker in each cell to restart the sort from without having to first look up the global marker in the cell it came from | |
| 16:05:51 | dansmith | but I think you said none of those are options without retooling the api stuff anyway, | |
| 16:05:55 | dansmith | so probably not worth it | |
| 16:06:23 | mdbooth | mriedem: In response, I could change the change id of my patch if you like? | |
| 16:06:42 | dansmith | I was hoping to not have to use the full instance_list approach if we had a much limited set of sort criteria and stuff, | |
| 16:06:54 | dansmith | but if we're going to implement the full keys/dirs approach at the bottom, then maybe we need to | |
| 16:07:14 | mdbooth | I'd forgotten it, tbh, but my patch is a whole lot more thorough. It deals with the transition to mandatory bdm uuid explicitly. | |
| 16:07:16 | dansmith | I hate to complicate the instance_list stuff by genericifying the bits we can do, especially if this is the only thing we'll ever need to use it for | |
| 16:07:16 | mriedem | mdbooth: ? just cleanup dan's old patches | |
| 16:07:38 | mriedem | mdbooth: adding uuids to things is a pretty standard formula now, we've done it in several other objects and talbes | |
| 16:07:40 | mriedem | *tables | |
| 16:07:46 | mriedem | so i'm not sure what more needs to be done | |
| 16:07:48 | dansmith | mdbooth: you can't add the unique constraint in that migration, AFAIK | |
| 16:08:03 | mdbooth | mriedem: Right, I cribbed off the migration uuid patch. | |
| 16:08:08 | mriedem | bdm.uuid must start as nullable | |
| 16:08:12 | mdbooth | mriedem: However, dansmith 's patch is way older than that | |
| 16:08:24 | mriedem | it can't be that far of | |
| 16:08:25 | mriedem | *off | |
| 16:08:38 | mriedem | because dan wrote the migration one, and the services one was based on the aggregates and compute nodes ones, which dan wrote as well | |
| 16:08:41 | mdbooth | mriedem: Yes, it does. However my patch ensures that uuid is always set on read | |
| 16:08:59 | mdbooth | Seriously, some bits of my patch are very similar to the old patch | |
| 16:09:06 | mdbooth | Some are cribbed off the newer stuff | |
| 16:09:12 | mdbooth | It amounts to the same thing | |
| 16:09:21 | dansmith | mdbooth: I thought you forgot about the old patch? :) | |
| 16:09:29 | dansmith | anyway, keeping the same review is important for historyand context, IMHO | |
| 16:09:29 | mdbooth | dansmith: I did :) | |
| 16:09:43 | mdbooth | I didn't forget about migration uuid, though | |
| 16:09:55 | mriedem | mdbooth: that was commented in here yes https://review.openstack.org/#/c/242603/23/nova/objects/block_device.py | |
| 16:10:01 | mdbooth | Most of the changes in there are pretty obvious, though | |
| 16:10:03 | mriedem | mdbooth: it's just a matter of cleaning up those old patches a bit | |
| 16:10:44 | mriedem | there are other reviews that have referenced those at this point, so i'd prefer to use those | |
| 16:10:49 | mriedem | plus, original author and all that | |
| 16:12:40 | mriedem | dansmith: "but if we're going to implement the full keys/dirs approach at the bottom, then maybe we need to" yeah that's where i'm at | |
| 16:12:46 | mriedem | i.e., | |
| 16:13:04 | mriedem | is it easier to just copy the instance_list stuff, or retool the marker encoding behavior for this api | |
| 16:13:47 | dansmith | well, there's three options I think: | |
| 16:13:49 | mdbooth | dansmith: Anyway, I would fix up the old patch by writing my patch instead. All the obvious bits are identical, and all the extra bits are required but not present in the older patch. | |
| 16:13:52 | dansmith | 1. retool instance_list to be usable by migrations | |
| 16:13:56 | dansmith | 2. Copy and modify | |
| 16:14:18 | dansmith | 3. Implement something limited from that approach that is slimmer and specific to migrations | |
| 16:14:21 | mdbooth | dansmith: Some bits of it are very similar, but we've written way more similar patches before :) | |
| 16:14:34 | mdbooth | I forgot about it. However, the work's now done. | |
| 16:14:42 | openstackgerrit | melanie witt proposed openstack/nova master: Remove old-style quotas code https://review.openstack.org/511689 | |
| 16:14:43 | openstackgerrit | melanie witt proposed openstack/nova master: Follow up on removing old-style quotas code https://review.openstack.org/524234 | |
| 16:14:58 | mdbooth | I can either throw the work away, or... not the work away. | |
| 16:15:09 | dansmith | huh? | |
| 16:15:29 | dansmith | my patch has history back to 2015 on it.. I just think it makes sense to do the uuid field and object change in that review so we keep that context | |
| 16:15:31 | dansmith | not sure why we wouldn't | |
| 16:15:48 | dansmith | I think that's all mriedem is saying, and I agree | |
| 16:16:00 | mdbooth | Ok, but those parts of my patch are so similar to your patch... | |
| 16:16:04 | mdbooth | What do you want me to do? | |
| 16:16:13 | mdbooth | Rebase my patch on your patch and subtract the older work? | |
| 16:16:17 | dansmith | your patch also puts the full online migration change in with the schema change, which just makes it bigger | |
| 16:17:03 | mdbooth | dansmith: There's a reason for that. | |
| 16:17:18 | mdbooth | Although I could split it. | |
| 16:17:32 | dansmith | what's the reason? | |
| 16:17:47 | mdbooth | I was thinking that you'd want to run the online migration asap after the schema change, because it's more efficient. | |
| 16:18:05 | dansmith | but that doesn't fit the way we do these, | |
| 16:18:17 | dansmith | because you have to be able to apply the schema change before rolling any code | |
| 16:18:22 | mdbooth | I can split it, it's no hassle. | |
| 16:18:23 | dansmith | if you land that schema change before any code to handle it, | |
| 16:18:31 | dansmith | then it kinda self-tests that you haven't broken anything | |
| 16:18:36 | dansmith | we've done a lot of these | |
| 16:18:42 | melwitt | mriedem: that's really weird what you're seeing in that func test. I'm gonna look into it more today. yesterday I didn't find what's going wrong yet | |
| 16:18:55 | melwitt | (what's going wrong with CellDatabases) | |
| 16:18:59 | mdbooth | dansmith: I'm talking about efficiency on a large deployment | |
| 16:19:11 | dansmith | um, me too? | |
| 16:19:24 | mdbooth | You run the schema change, you roll the code, then you want to run the online migration asap | |
| 16:19:33 | mdbooth | But you don't have to, it's just more efficient if you do | |
| 16:19:46 | mdbooth | Anyway, I can split that out | |