| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-21 | |||
| 19:35:02 | dansmith | yeah, so all the more reason to let it do the cell and memory check in one go of select_destinations right? | |
| 19:35:23 | mriedem | yeah, although i don't see where in the scheduler we check that the requested destination is in the same cell... | |
| 19:35:44 | mriedem | get_host_states_by_uuids ? | |
| 19:36:06 | mriedem | https://github.com/openstack/nova/blob/16.0.0.0rc1/nova/scheduler/host_manager.py#L635 | |
| 19:36:33 | dansmith | yeah | |
| 19:36:52 | mriedem | cool, ok, so i think this narrows things a bit | |
| 19:37:09 | mriedem | i think we are missing some stuff in the request spec backport to legacy format stuff, but i'm not messing with that | |
| 19:37:13 | mriedem | those are all bauzas questions | |
| 19:37:25 | mriedem | since it seems there are 4 ways to construct a request spec for the scheduler | |
| 19:39:03 | dansmith | yeah that whole mess is still...a mess | |
| 19:39:26 | mriedem | well it's 2:40pm on my first day back and i'm already knee deep in a mess that has to be fixed for rc2 | |
| 19:39:27 | mriedem | yay! | |
| 19:40:23 | dansmith | well, | |
| 19:40:42 | dansmith | presumably we _could_ just document and fix post release if needed | |
| 19:40:48 | dansmith | if it helps | |
| 19:40:53 | mriedem | yeah i thought about that | |
| 19:40:57 | mriedem | known issue and all | |
| 19:41:00 | mriedem | "known regression" | |
| 19:41:16 | mriedem | "at least we let you know about it beforehand, you're welcome" | |
| 19:41:21 | dansmith | we wouldn't want to wait long, of course, but.. | |
| 19:41:29 | dansmith | things could be worse | |
| 19:41:33 | mriedem | yeah | |
| 19:41:38 | mriedem | i.e. optional api | |
| 19:42:26 | mriedem | ok i'll wip something together and put it on top of https://review.openstack.org/#/c/495170/ and see what happens | |
| 19:43:04 | dansmith | okay | |
| 19:43:56 | openstackgerrit | Merged openstack/nova master: Correct statement in api-ref https://review.openstack.org/495724 | |
| 19:54:54 | mriedem | this was the other bug, I haven't triaged it yet https://bugs.launchpad.net/nova/+bug/1712045 | |
| 19:54:55 | openstack | Launchpad bug 1712045 in OpenStack Compute (nova) "nova doesn't clean up the resources after live migrate" [Undecided,New] | |
| 19:55:43 | mriedem | looks similar to what was needed with evacuate though https://review.openstack.org/#/c/494625/1/nova/compute/manager.py | |
| 19:55:51 | mriedem | remove the allocations for the instance and the source node | |
| 19:56:46 | dansmith | yeah | |
| 19:58:17 | dansmith | mriedem: that test for the bug you're working on has the post assertion for the destination's allocations commented out, | |
| 19:58:30 | dansmith | so fixing 1712045 will be required for that one too I think | |
| 20:06:02 | openstackgerrit | Merged openstack/nova master: doc: Address review comments for contributor index https://review.openstack.org/491517 | |
| 20:07:10 | mriedem | wtf, is local docs build blowing up a known issue? | |
| 20:07:25 | mriedem | https://gist.github.com/mriedem/4a6dcb52ed867af14f16989d7b83739a | |
| 20:18:40 | openstackgerrit | Matt Riedemann proposed openstack/nova master: doc: Address review comments for main index https://review.openstack.org/492645 | |
| 20:19:31 | mriedem | dansmith: i think we should probably get ^ to rc2 to fix the "OpenSack" thing in the first section you read in nova's docs | |
| 20:19:53 | dansmith | lol | |
| 20:26:45 | openstackgerrit | Ilya Popov proposed openstack/nova master: Tests: Add cleanup of 'instances' directory https://review.openstack.org/491589 | |
| 20:32:50 | mriedem | as for that live migration test, yeah it's a mix of both bugs, and the comments are wrong for some of the existing test | |
| 20:32:55 | mriedem | i'll update that test also | |
| 20:32:58 | dansmith | mriedem: well, I have the change made for the post-migration update I think, but I can't really use it until I have your patch to create the doubled allocation in the scheduler | |
| 20:33:11 | mriedem | cleaning up the test atm | |
| 20:33:12 | dansmith | otherwise I'm trying to push empty allocations to placement | |
| 20:33:16 | dansmith | no problem, just FYI | |
| 20:38:56 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional live migrate test https://review.openstack.org/495811 | |
| 20:38:57 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional force live migrate test https://review.openstack.org/495170 | |
| 20:45:25 | mriedem | ok +2 on both of the live migration functional tests | |
| 21:14:16 | mriedem | i was thinking that with setting RequestSpec.requested_destination we might end up bypassing the filtering for MEMORY_MB in placement, but looks like we do that first regardless of the requested/forced host | |
| 21:14:37 | mriedem | so we always call to placement to get the candidates, and then wittle that down based on forced hosts, and then further filter that using the filters | |
| 21:14:59 | mriedem | so we should be ok with removing that ram check in the live migration task in conductor | |
| 21:23:52 | cfriesen_ | mriedem: looking at your comments for https://bugs.launchpad.net/nova/+bug/1712008 I guess that actually calling the scheduler will end up filtering for cells as well? | |
| 21:23:53 | openstack | Launchpad bug 1712008 in OpenStack Compute (nova) pike "Force live migrate doesn't claim resources on the target host" [Critical,Triaged] | |
| 21:24:09 | mriedem | nope | |
| 21:24:20 | mriedem | because conductor doesn't set request_spec.requested_destination.cell | |
| 21:24:25 | mriedem | in the LiveMigrateTask | |
| 21:38:35 | cfriesen_ | mriedem: that bug is in the context of forcing a host...how is it even valid to claim resources when forcing a host? It could end up consuming resources that aren't available. | |
| 21:39:10 | cfriesen_ | mriedem: (when factoring in overcommit etc) | |
| 21:39:50 | mriedem | cfriesen_: pre-placement you'd end up claiming resources in the compute anyway | |
| 21:39:52 | mriedem | via the resource tracker | |
| 21:39:55 | mriedem | yo'ud just fail much later | |
| 21:40:38 | mriedem | but let me verify that first | |
| 21:42:57 | mriedem | hmm, yeah we don't call the resource tracker to make a claim during live migration... | |
| 21:43:33 | cfriesen_ | mriedem: that's part of the patch series that's been under review forever | |
| 21:44:00 | mriedem | so, we don't want to build more on claims in the computes, yes? | |
| 21:44:09 | mriedem | because the RT is a mess | |
| 21:44:41 | mriedem | and we are moving things to the scheduler so we can make more accurate decisions when building instances up front, rather than rely on reschedules | |
| 21:44:42 | mriedem | yes? | |
| 21:45:15 | cfriesen_ | As I understand it we want to do the claims from the scheduler. I'm not sure what meaning "force" has when it comes to things like cpu pinning, hugepages, PCI devices, etc. | |
| 21:48:38 | mriedem | force from the api just meant use this host i'm giving you | |
| 21:48:51 | mriedem | rather than leave it up to the scheduler to pick one randomly | |
| 21:49:35 | mriedem | now reading https://docs.openstack.org/nova/latest/reference/api-microversion-history.html#id27 this could be considered an API change | |
| 21:49:37 | cfriesen_ | mriedem: you can specify which host you want without using "force"...."force" just means "not verifying the provided destination host by the scheduler". | |
| 21:49:40 | mriedem | "Also changes the live-migrate action behaviour when providing a host string field by calling the nova scheduler to verify the provided host unless the force attribute is set." | |
| 21:49:49 | mriedem | yeah point taken | |
| 21:50:19 | mriedem | so if the point of the force flag is to not call the scheduler, because you don't care about filters or anything, | |
| 21:50:22 | cfriesen_ | so if we passed the force option to the scheduler, and if set we skip all the filters/weighters but still do the claim | |
| 21:50:42 | mriedem | but we want to call the scheduler to do the allocation on the dest host, then we have a conflict | |
| 21:51:18 | mriedem | and we don't want to do the allocation on the dest host from conductor, since that triplicates that behavior between nova-compute, nova-scheduler and nova-conductor | |
| 21:51:21 | cfriesen_ | I think we could get the spirit of the "force" option by still calling the scheduler but skipping the filters/weighers | |
| 21:52:01 | cfriesen_ | that would allow us to avoid additional duplication | |
| 21:52:06 | mriedem | that probably involves passing yet another ignore/force type thing to the scheduler | |
| 21:52:09 | cfriesen_ | yea | |
| 21:52:19 | mriedem | request spec already has ignore_hosts and force_hosts | |
| 21:52:23 | mriedem | and requested_destination | |
| 21:52:40 | mriedem | this would be, ignore source host, force dest host, but seriously, really ignore and force everything wink wink | |
| 21:52:47 | cfriesen_ | hmm...what's the difference between force_hosts and requested_destination? | |
| 21:52:56 | mriedem | cfriesen_: see the email i just send to the ML :) | |
| 21:53:13 | cfriesen_ | lol | |
| 21:54:12 | cfriesen_ | right. maybe we can reuse force_hosts to tell it to skip the filters/weighers but still do claims | |
| 21:54:35 | mriedem | no that gets really confusing | |
| 21:54:47 | mriedem | adds more confusion to an already really confusing thing i mean | |
| 21:54:55 | cfriesen_ | I think "ignore_hosts" is more than just ignore source...it's also used to keep track of hosts we've already tried (when doing a reschedule) | |
| 21:55:03 | mriedem | like, i think force_hosts is used today in the scheduler hint for picking a specific host using scheduler hints | |
| 21:55:08 | mriedem | and that goes through the filters/weighers | |
| 21:55:13 | mriedem | yes it is | |
| 21:55:31 | cfriesen_ | the joys of an organically grown API. :) | |
| 21:55:32 | mriedem | so if we pass something to the scheduler, i'd say it's an explicit new flag, called skip_filters or something | |
| 21:55:44 | cfriesen_ | yeah, that's probably cleanest | |