Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-11-30
15:17:26 mriedem but i can replay this on top of your fixture cleanup patch and see if that fixes it
15:18:04 mriedem mdbooth: ?
15:18:35 mriedem mdbooth: if i added a comment it's because i oculdn't understand ndipanov's code and had to figure it out at one point, and left a comment for posterity
15:19:02 mdbooth mriedem: Not important. I was doing something else and came across it. It made my head hurt :) I submitted this throw-away: https://review.openstack.org/#/c/524208/
15:20:09 mdbooth I assume this was very X of this patch for some large value of X, and this complexity made sense at some version <X
15:20:16 mdbooth s/very/version/
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,831 ERROR [nova.tests.functional.test_servers] Failed to find host host2 in cell cell2
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: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 mriedem mdbooth: ? just cleanup dan's old patches
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: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 mdbooth dansmith: I did :)
16:09:29 dansmith anyway, keeping the same review is important for historyand context, IMHO
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

Earlier   Later