Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-11
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 - the bottom where I'll flip from -2 to +2 when we're ready
19:21:50 efried - https://review.opendev.org/#/c/678455/ where I'll proxy a vote from sean-k-mooney
19:21:50 efried stephenfin, sean-k-mooney, luyao, alex_xu: I'm +2 on everything vpmem except:
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
19:50:15 mriedem he added handling for BuildAbortException,
19:50:19 sean-k-mooney the finally block will be run by the way when we catch the BuildAbortException
19:50:27 sean-k-mooney i was just comming that in gerrit
19:50:29 mriedem but was missing some things we do when we don't reschedule from a prep_resize failure, like recording a fault and sending an error notification
19:56:17 gibi mriedem: I'm not sleeping yet but I'm already short on brainpower
19:56:40 mriedem gibi: ok if you can look at https://review.opendev.org/#/c/676980/20 before going to bed i can try to update it
19:56:49 mriedem but it's looking non-trivial
19:57:04 mriedem otherwise i'll just move up the series since those changes look simple
19:59:25 gibi now I read your comments. it seems non trivial indeed. Would it be too bad letting the re-schedule happen as a first solution. And optimizing out the unnecessary rechedule later?
20:01:13 gibi mriedem: I cannot implement the functional test and the proper cleanup today. I can do that tomorrow morning. Or If you able to implement it then I can try to review it
20:02:36 sean-k-mooney donnyd: the new lable job passed https://review.opendev.org/#/c/680738/ so im going to call it a day
20:03:57 gibi I will be back in 15 minutes
20:05:12 mriedem gibi: i could let the BuildAbortException handling slide to a follow up but the fact that i can comment out the new code and none of the functional tests fail is kind of a blocker for me
20:05:39 gibi mriedem: ack, that was a surprise for me too.
20:06:22 gibi mriedem: I suggest you move forward with the reviews and I will respin this in the morning with a proper functional test
20:07:05 gibi mriedem: as I run out of time today and I don't want to ask you to troubleshoot those functional tests
20:07:40 openstackgerrit Luyao Zhong proposed openstack/nova master: Add functional tests for virtual persistent memory https://review.opendev.org/678470
20:08:29 mriedem gibi: yup sounds like a plan
20:11:44 openstackgerrit Merged openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072
20:21:37 gibi shover thoughts: the computes in those tests are symmetrical. they have the same PF setup. PFs are named the same on host1 and on host2 and we are "updating" the PF name in the PCI request.

Earlier   Later