| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-11 | |||
| 19:05:38 | mriedem | ok so hear me out, | |
| 19:05:38 | sean-k-mooney | and backport a config option to opt out | |
| 19:05:44 | mriedem | the rebuild api takes new metadata for the server, | |
| 19:06:00 | dansmith | no | |
| 19:06:02 | dansmith | no | |
| 19:06:05 | mriedem | what if we have a skip_numa_topology_filter metadata that the user can pass in?! | |
| 19:06:09 | mriedem | and then the filter checks for that | |
| 19:06:11 | mriedem | and it's great | |
| 19:06:11 | dansmith | noooooooooo | |
| 19:06:13 | mriedem | problem solved | |
| 19:06:38 | mriedem | ok so the image takes random metadata, | |
| 19:06:39 | mriedem | so hear me out | |
| 19:06:57 | mriedem | os_skip_numa_filter_but_only_for_rebuild=true | |
| 19:07:04 | mriedem | we'll fix it in U | |
| 19:07:12 | dansmith | Well, officer, you should have heard what he was suggesting before I shot him... Hear me out... | |
| 19:07:23 | mriedem | that's justifiable homicide in some states | |
| 19:07:34 | dansmith | yeah | |
| 19:07:40 | artom | We fix it? No, U fix it | |
| 19:07:52 | sean-k-mooney | well the custromer has validated that tehy never us the image to change numa toplogy | |
| 19:07:53 | mriedem | get in line with bauzas and the bad jokes | |
| 19:08:03 | sean-k-mooney | so for them that skip on rebuild is valid | |
| 19:08:13 | sean-k-mooney | but i have aded https://bugs.launchpad.net/nova/+bug/1763766 to the downstrema bug | |
| 19:08:14 | openstack | Launchpad bug 1763766 in OpenStack Compute (nova) "nova needs to disallow resource consumption changes on image rebuild" [Medium,Triaged] | |
| 19:08:28 | sean-k-mooney | so what i am going to propose is i implemnt a check for that | |
| 19:08:44 | sean-k-mooney | and if and only if that passes we skip the numa toplogy filter | |
| 19:08:54 | sean-k-mooney | becasue not numa things changed | |
| 19:09:03 | sean-k-mooney | on rebuild only | |
| 19:09:54 | sean-k-mooney | and like the check we have to prevent numa migratio nwe can backport the saftey check for no resouce change on rebuild | |
| 19:10:17 | sean-k-mooney | and give peole a workaofund config option to disable it if they want to shot them selves in the foot | |
| 19:11:11 | sean-k-mooney | i have aslo said that if we dont fix it upstream im not fixing it downstream only | |
| 19:13:37 | sean-k-mooney | mriedem: dansmith does ^ sound fair. fixing https://bugs.launchpad.net/nova/+bug/1763766 is a prequisite for fixing https://bugs.launchpad.net/nova/+bug/1804502 safely | |
| 19:13:39 | openstack | Launchpad bug 1763766 in OpenStack Compute (nova) "nova needs to disallow resource consumption changes on image rebuild" [Medium,Triaged] | |
| 19:13:40 | openstack | Launchpad bug 1804502 in OpenStack Compute (nova) "Rebuild server with NUMATopologyFilter enabled fails (in some cases)" [Undecided,In progress] - Assigned to David Hill (david-hill-ubisoft) | |
| 19:14:03 | eandersson | Would it be possible to add an option to allow rebuilds to re-schedule the VM? | |
| 19:14:23 | eandersson | something like openstack server rebuild <id> --migrate/reschedule/bla | |
| 19:14:28 | sean-k-mooney | like evauate | |
| 19:14:46 | eandersson | I can't reproduce this as well | |
| 19:15:26 | sean-k-mooney | rebuild is ment to be posiblein virt driver that dont support migration | |
| 19:16:29 | sean-k-mooney | there is noting in principal that would disallow a driver form having rebiuld be to a different host like a cold migration but it is not how its currelty impelmeted | |
| 19:16:49 | sean-k-mooney | we would have to change the noop claim to a move claim if the inplcem rebuild did not succesed | |
| 19:17:29 | sean-k-mooney | or expose it at the api like you suggest and make the resuling to a different host user contolerale | |
| 19:18:52 | sean-k-mooney | ingoring numa for a second since image can have tratis we need to check with placmenet in anycase to see it the traits on the new image are valid on the current host | |
| 19:20:44 | sean-k-mooney | eandersson: what is the behavior you would like to see | |
| 19:21:11 | eandersson | Better error message at least :p | |
| 19:21:40 | eandersson | But not sure what the ideal would be. | |
| 19:21:50 | efried | stephenfin, sean-k-mooney, luyao, alex_xu: I'm +2 on everything vpmem except: | |
| 19:21:50 | efried | - https://review.opendev.org/#/c/678455/ where I'll proxy a vote from sean-k-mooney | |
| 19:21:50 | efried | - the bottom where I'll flip from -2 to +2 when we're ready | |
| 19:21:53 | eandersson | Maybe just documentation on how to avoid this on image builds. | |
| 19:22:02 | efried | I'm now going to sleep. | |
| 19:22:06 | eandersson | It would be nice as an admin to have a way to fix it at least. | |
| 19:22:33 | sean-k-mooney | eandersson: well you cant avoid it if the instance has a numa toplogy | |
| 19:22:44 | sean-k-mooney | you can work around it via cold/live migation | |
| 19:22:50 | sean-k-mooney | but that it | |
| 19:23:13 | eandersson | Maybe always force reschedule on rebuild when numa is a thing | |
| 19:23:37 | sean-k-mooney | rebuild are not allowed to change host today | |
| 19:23:42 | sean-k-mooney | so there is no rescudler | |
| 19:24:03 | sean-k-mooney | whats actully happening is the numa toplogy filter is treating the rebuild like a new isntance spawn | |
| 19:24:09 | sean-k-mooney | so you need double the resouces | |
| 19:24:26 | eandersson | oh so that is why some of them worked | |
| 19:24:32 | sean-k-mooney | yep | |
| 19:24:49 | sean-k-mooney | if the instance does not have a numa toplogy or there is enough space it works | |
| 19:25:35 | sean-k-mooney | we can actully now tell if its a rebuild in the filter code but we have not added special handeling for that in the numa toplogy filter | |
| 19:26:17 | sean-k-mooney | the correct thign to do is to check if the numa topldoy woudl change. reject the rebuild if its true or skip the numa toplogy filter if not | |
| 19:26:34 | sean-k-mooney | that is more or less want i want to code up | |
| 19:28:03 | sean-k-mooney | if posible i would like to do the "rebuild chage numa toplogy" check at the api and return an expcti error | |
| 19:28:25 | sean-k-mooney | but i have not figured out all the pices yet | |
| 19:31:01 | mriedem | sean-k-mooney: if rebuild + new image + numa sometimes works if the new image numa topo fits on the same host, why wouldn't you just let the scheduler still try to fit it? | |
| 19:31:16 | mriedem | and only outright skip if the image is new + rebuild + none of the numa properties changed | |
| 19:31:30 | sean-k-mooney | mriedem: we dont update the instance numa toplogy in the db | |
| 19:31:35 | sean-k-mooney | because that is dont in the claim | |
| 19:31:37 | mriedem | and we don't claim | |
| 19:31:39 | sean-k-mooney | and we do a noop claim | |
| 19:31:43 | mriedem | yeah ok | |
| 19:31:52 | sean-k-mooney | so if we fixed that too then sure | |
| 19:31:54 | mriedem | so like new image + volume-backed server we just fail fast in the api today | |
| 19:32:21 | sean-k-mooney | thats becasue we dont supprot volume backed rebuild in general? | |
| 19:32:27 | mriedem | no, | |
| 19:32:39 | mriedem | it's because we don't replace the root volume with the new image | |
| 19:33:09 | mriedem | there are approved cinder/nova specs to support all of that in the past and cinder did a bunch of work to plumb a new api for us, but the nova side changes all stalled | |
| 19:33:41 | mriedem | gibi_bus: did you go to bed yet? | |
| 19:34:52 | sean-k-mooney | this liekly will become a spec in anycase. but what i wanted to do was first do the api/filer check to block it. then explore either updating the topology on rebuld and validatin git or otherwise having a more ocmplet fix | |
| 19:35:17 | sean-k-mooney | if it could fix then i would liek to allow it | |
| 19:35:32 | sean-k-mooney | but only if the toploy you ask for in the image is what you actlly end up with | |
| 19:35:48 | sean-k-mooney | which is not what would happen without updating the laim | |
| 19:35:51 | sean-k-mooney | *claim | |
| 19:37:40 | sean-k-mooney | the otherthing with allowing the toplogy to change on a rebuild would be keeping placmenet in sysnc if/when we model numa in placmenet | |
| 19:38:37 | sean-k-mooney | i dont want to complicate numa in plamcent more by having to deal with this at the same time | |
| 19:40:34 | mriedem | i could use some opinions on alternatives i've proposed for some new error handling during prep_resize in gibi's bottom unapproved change https://review.opendev.org/#/c/676980/20/nova/compute/manager.py@4529 which is the only thing holding it up right now | |
| 19:40:49 | mriedem | since i'll probably be the one implementing them today so we can still meet FF for the bw provider migration stuff | |
| 19:42:01 | mriedem | sean-k-mooney: yeah i'd focus on whatever you can that is backportable first | |
| 19:43:41 | sean-k-mooney | yep thats the plan. its in our team backlog for U and ill likely volenterr to be the person to fix it so im going to fix what can be backport first then consider how to imporve it later. | |
| 19:43:56 | sean-k-mooney | thanks for the link to the other bug by the way | |
| 19:44:03 | sean-k-mooney | i forgot that was still open | |
| 19:45:06 | openstackgerrit | Merged openstack/nova master: Make _revert_allocation nested allocation aware https://review.opendev.org/676138 | |
| 19:45:07 | sean-k-mooney | im jsut looking at https://review.opendev.org/#/c/676980/20/nova/compute/manager.py@4529 | |
| 19:45:42 | sean-k-mooney | i dont really have context right now but what alternitve were you tinking of | |
| 19:49:32 | mriedem | they are listed below | |
| 19:49:57 | sean-k-mooney | ok ill keep reading | |
| 19:50:06 | mriedem | gibi added code to the prep_resize flow that can result in a BuildAbortException getting raised and in that case we shouldn't try to reschedule | |