Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-11-30
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
16:19:47 dansmith how does that affect whether the patch is large or small?
16:20:00 mdbooth My change also makes the field non nullable in the object
16:20:30 mriedem umm
16:20:33 dansmith what does that, or the other things have to do with re-using the review for context?
16:20:36 mriedem https://review.openstack.org/#/c/242603/23/nova/objects/block_device.py is already non-nullable
16:21:06 mriedem mdbooth: seriously just invest the time on splitting up your duplicate change and just rebase and address gaps in the older patches
16:21:34 mdbooth mriedem: Ok. It does seem like a bunch of busy work for little gain, though.
16:21:42 mdbooth I just forgot about the old review.
16:22:07 mdbooth I'm going to have to reconcile any minor changes with a bunch of code I've already written and tested.
16:22:41 mriedem how about i take over dan's patches then
16:22:47 mriedem will everyone be happy with that solution?
16:23:10 dansmith mriedem: so I'm going to try to strip out the instanceness of the list routines and see how bad that is in order to make a call on the migration stuff
16:23:16 mdbooth mriedem: No, because I'm working on this actively right now, including a ton of follow-on
16:23:24 mdbooth If anybody's going to do the busy work, it needs to be me
16:23:28 mdbooth It's just a pita
16:33:04 AJaeger nova cores, please review https://review.openstack.org/520373 to update relesaenotes to allow infra to improve the job
16:34:03 AJaeger yummy
16:37:24 mriedem melwitt: i was doing something wrong in my assertion code
16:37:33 mriedem i wasn't using the targeted context to query the compute nodes out of the cell
16:37:36 mriedem seeing if that changes tihngs
16:38:24 stephenfin AJaeger: -1 for use of double quotes ;)
16:38:30 stephenfin AJaeger: I kid. +2
16:38:45 AJaeger stephenfin: ;9
16:40:03 mriedem nope still fails
16:40:06 mriedem host2 shows up in cell1
16:41:21 melwitt sad town :(
16:41:46 mriedem looking at CellDatabasesTestCase i'm not sure why this doesn't work
16:41:50 mriedem except,
16:42:02 mriedem when the compute node record is created, it's via the start of the service,
16:42:07 mriedem which has an untargeted context,
16:42:16 mriedem so it's going to create the service/computenode in the default context, which is cell1
16:42:18 mriedem so i guess that's why
16:42:46 mriedem to hack around this, i'd have to create the compute node records in the cells before actually starting the services
16:43:00 mriedem or allow temporarily mutating the default context in the CellDatabase fixture
16:43:21 openstackgerrit Eric Fried proposed openstack/nova master: Nix log translations from scheduler.client.report https://review.openstack.org/524245
16:43:27 melwitt well, what I did in my patch is wrap the Service class and target the context during start
16:44:02 melwitt if that's not working then I messed up what I tried to do
16:46:17 kashyap dansmith: When you get a moment, maybe you'd know top off your head -- during 'evacuate', certainly CPU model compatibility is checked before an instance is moved away, yes?
16:46:45 dansmith kashyap: if we run the scheduler then I assume so
16:46:45 mriedem kashyap: would depend on the scheduler filters
16:46:56 mriedem unless, of course, you specify a host with the force option
16:46:56 dansmith if a host is provided, I don't think so
16:47:02 mriedem then we bypass the scheduler altogether
16:47:14 mriedem it's not the same
16:47:16 mriedem evacuate is a move
16:47:16 kashyap mriedem: Yep, scheduler filters is what I was going toc heck
16:47:17 mriedem rebuild is not
16:47:42 mriedem you can evacuate with a target host and if you don't specify the force flag, we'll validate that host against the scheduler filters,
16:47:50 mriedem if you specify a host + force, we don't run through the scheduler filters
16:47:59 mriedem because you've decided you want to shoot yourself in the foot
16:48:04 mriedem and we hand you the gun to do that
16:48:47 kashyap dansmith: mriedem: Do you have a pointer to the relevant Scheduler code? libvirt / QEMU has reworked how CPU models are detected, so I'm trying to investigate what action items are there on the Nova side
16:48:55 mriedem if you rebuild (not evacuate) with a new image, well, then it depends on which roll of the cve dice you've landed on
16:48:55 kashyap mriedem: Heh, noted
16:49:15 mriedem kashyap: the compute capabilities filter
16:49:30 mriedem https://github.com/openstack/nova/blob/master/nova/scheduler/filters/compute_capabilities_filter.py
16:49:44 mriedem i think anyway
16:49:53 mriedem i'm not super well versed in the details of what all the filters do
16:49:58 mriedem bauzas would know
16:50:13 kashyap Wauw, thanks for the super quick response
16:50:35 kashyap dansmith: Yes, good point on the "if you provide a host, then no checks are performed"
16:50:56 kashyap Because, well, if you're providing a target host yourself, then you're expected to keep an eye on CPU comapt
16:51:03 kashyap s/comapt/compat/
16:55:43 kashyap mriedem: I think the scenario where one would "specify a host + force" is the admin damn sure knows about his target host, and wants to avoid extra checks?
16:56:21 mriedem wants to avoid the scheduler kicking out the specified host, yes
16:56:25 edleafe kashyap: yes, that was the idea
16:56:34 edleafe "I'm the admin, dammit!"

Earlier   Later