Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-10-18
15:00:18 mriedem because the inventory gets updated via the RT periodic
15:01:00 mriedem so if i just set a custom rc on my ironic node, the periodic runs, it detects the new rc and will update inventory, but while in the driver we're migrating the instance because it's node has an rc now, if we try updating the allocation for that rc before the node RP inventory has that rc, it will fail with a 409
15:01:06 johnthetubaguy mriedem: dang, yeah
15:01:06 mriedem "409 Conflict if there is no available inventory in any of the resource providers for any specified resource classes or inventories are updated by another thread while attempting the operation."
15:01:29 johnthetubaguy mriedem: I guess we skip adding it as migration complete, and go around again
15:01:41 dansmith mriedem: is comment #1 what I should read?
15:01:50 mriedem dansmith: or 2
15:02:05 mriedem oh yeah 1 is my description of the problem
15:02:09 mriedem in 'merican
15:02:17 johnthetubaguy mriedem: I have a related thing here, I am not keen on what it could do across upgrade mind: https://review.openstack.org/#/c/513001
15:02:28 johnthetubaguy heh
15:03:03 dansmith mriedem: this is because pike doesn't do healing of allocations right?
15:03:22 johnthetubaguy dansmith: +1
15:03:28 mriedem yes
15:03:41 mriedem the flavor migration code has a comment asserting that the allocations are updated via the RT periodic
15:03:55 mriedem probably b/c it was written 1/2 a day before that changed :)
15:04:06 dansmith having the virt driver do that thing we decided was bad to have compute (at all) do, seems like a bad call to me,
15:04:13 dansmith although I understand something needs to do it
15:04:54 dansmith it'd be nice if we had some way to let the driver signal to compute that something fundamental has changed about the instance to force the heal,
15:04:58 mriedem could we get the scheduler to do it? probably not b/c the issue during scheduling isn't with the instance already consuming that node, it's a new build request and the scheduler thinking the node is free
15:05:11 mriedem dansmith: that's what i was thinking as an option above,
15:05:23 mriedem have a way for the RT to call into the driver to ask if the allocatoin should be forced
15:05:36 dansmith yeah
15:05:52 mriedem that might also fix the issue with trying to update the allocatoin before we've posted inventory for the newly set node.rc
15:05:54 dansmith which could apply to other virt drivers that have things happen underneath them (like vmware moving an instance within a cluster)
15:06:00 johnthetubaguy I am missing the trigger, a new build fails and triggers the refresh?
15:06:12 jaypipes stephenfin: k, done on the pci numa policies spec.
15:06:27 mriedem johnthetubaguy: where does the new build fail? scheduler or compute?
15:06:34 mriedem i thought it was compute and triggered a retry
15:06:50 johnthetubaguy yeah, its on the compute, ironic find the node is already in use by another instance
15:06:57 mriedem so spawn fails?
15:07:01 johnthetubaguy yeah
15:07:16 johnthetubaguy I should double check the logs on that
15:07:21 stephenfin jaypipes: Thank you sir. Much appreciated, as always
15:07:51 jaypipes stephenfin: no problem. or however one says no problem in Gaelic.
15:09:05 mriedem bauzas: you were +2 on https://review.openstack.org/#/c/361140/ before so are you still good
15:09:05 mriedem ?
15:09:23 efried An dtugann stephenfin Gaeilge? I was kind of guessing not...
15:10:15 mriedem johnthetubaguy: not sure i like the spawn fail / refresh thing,
15:10:28 mriedem trying to think of how we can set a flag for instances that have migrated but need their allocations updated by the RT
15:10:55 mriedem including instances that have had their flavors migrated already
15:11:06 dansmith the problem being we do the migration async right?
15:11:24 dansmith we come up and before we can do all of them we might get a boot request?
15:11:25 mriedem ah crap didn't think about that either
15:11:46 dansmith we could disable ourselves until the migration is complete
15:12:22 mriedem the async migration also means that if the RT asked the driver for any instances to forcefully update allocations, they might not be set yet, but the next pass or the periodic would hit them
15:12:32 mriedem i was thinking the driver is shoving stuff onto a work queue,
15:12:36 mriedem the RT pulls off that queue
15:12:50 mriedem once all instances are processed the queue remains empty
15:13:09 johnthetubaguy get_inventory, could we do the migration on that call, and return something if the allocation needs refreshing?
15:13:16 dansmith if we disable ourselves we can still do management operations of existing instances, and the scheduler won't send us new stuff until we're ready
15:13:32 dansmith johnthetubaguy: that's kinda icky
15:13:39 dansmith johnthetubaguy: I mean, really icky
15:13:50 dansmith johnthetubaguy: gross even
15:14:04 johnthetubaguy worse than virt driver calling to placement?
15:14:15 dansmith also recall that people can do this before anything starts back up during an upgrade
15:14:52 openstackgerrit Balazs Gibizer proposed openstack/nova master: Transform instance.exists notification https://review.openstack.org/403660
15:14:53 openstackgerrit Balazs Gibizer proposed openstack/nova master: Add sample test for instance audit https://review.openstack.org/480955
15:15:04 dansmith johnthetubaguy: it's just overloading get_inventory() as a general hook to do potentially a ton of db maintenance
15:15:11 johnthetubaguy so the alternative is dropping the resources:VCPU=0 in the new flavor
15:15:13 dansmith johnthetubaguy: it'd be better to just provide a proper hook I think
15:15:58 johnthetubaguy dansmith: I am agreeing its horrid, just thinking relative horrid here really
15:16:10 dansmith johnthetubaguy: it's relatively horrid yes :)
15:16:21 dansmith johnthetubaguy: you don't like the self disable?
15:16:37 dansmith if we do it in get_inventory we still have a race right?
15:16:41 dansmith with the scheduler
15:16:52 johnthetubaguy so its a race with when the flavor gets updated I guess
15:17:45 mriedem i'm not sure at which point you auto-disable
15:17:57 dansmith mriedem: on init_host(), if there are unmigrated instances
15:18:15 dansmith and then re-enable when you're done with the migration
15:18:44 johnthetubaguy but if the resource class isn't set on the node, we are never done
15:18:44 mriedem so originally when we did the flavor migration it was on init_host and then that moved because of the hash ring stuff
15:19:05 dansmith johnthetubaguy: that had to be set before queens, right?
15:19:09 mriedem yeah i'm trying to think if we re-enable too soon, or disable too long
15:19:29 dansmith so queens starting up can assume it's set no?
15:19:35 johnthetubaguy dansmith: yes, but this is really about making pike work
15:20:05 johnthetubaguy so right know I can't update the flavors in pike, for them to be set in time for queens, because of this bug
15:20:09 dansmith oh, was this migration in pike? I'm misremembering the timing
15:20:23 johnthetubaguy yeah, sadly
15:20:39 mriedem yeah we have to backport the fix
15:21:05 dansmith johnthetubaguy: but you can set the node class before pike, and you can run the migration before you start anything else up right/
15:21:07 mriedem so i'm still thinking queue and hook callback from RT to driver
15:21:51 dansmith mriedem: explain your queue thing again?
15:22:18 mriedem so before https://github.com/openstack/nova/blob/e11a8687aef527eee9f7c733db6cccd5b902afbb/nova/virt/ironic/driver.py#L523 we're going to have to see if the instance already has an allocation for the normalized_rc,
15:22:22 cdent does anyone have a reference to previous discussion/decisions on why we don’t do regular allocation updates? (something I can read, rather than taking us off track here)
15:22:28 mriedem if it doesn't, we have to put the instance/rc into a queue,
15:22:52 mriedem when the RT periodic runs, it calls into the driver to say, give me any instances that need their allocations updated and we'd dequeue at that point
15:23:38 mriedem then ^ should auto-heal during the periodic as the admin is setting node rcs in ironic
15:23:40 mriedem while things are running
15:24:27 mriedem cdent: https://review.openstack.org/#/c/491012/
15:24:43 cdent thanks
15:24:50 dansmith mriedem: you mean the first time we run the RT periodic we end up migrating all the instances the driver needs before we update inventory?
15:25:18 mriedem cdent: in a nutshell, in pike we want the scheduler to create allocations, including doubling them up for moves and shared providers (before we stopped trying to make shared providers work in pike) - the problem is the periodic task in ocata computes will overwrite the allocations created by the scheduler
15:25:39 mriedem dansmith: need to see if the RT updates inventory before allocations
15:26:14 mriedem it doesn't
15:26:15 mriedem shit
15:26:24 mriedem the RT would update allocations before inventory
15:26:35 dansmith even still, we run most of that code at other times than just the perioidic
15:27:47 mriedem the allocatoin update does happen during instance_claim
15:27:55 dansmith mriedem: so we still do allocation updates if we have ocata computes.. what if we keep doing them if we have ocata nodes and the driver says we need to?

Earlier   Later