Earlier  
Posted Nick Remark
#openstack-nova - 2019-01-15
14:27:31 sean-k-mooney stephenfin: in either case i think the code you have is correct
14:29:33 sean-k-mooney can_fit_pagesize will only consider teh total or availbe memory for the specified page size which will always be less or equal to the total or available ram for the numa node
14:30:33 alex_xu emm...the problem is overcommit, not less or equal to the total or avail ram
14:31:20 sean-k-mooney this code is only ment to ensure the vm does not request more ram then is available for that pagesize on that numa node
14:31:53 sean-k-mooney over commit from multiple vms is intended to be allowed for the smallest page size as we allow ram allocation ratios > 1
14:34:23 sean-k-mooney alex_xu: i take it that your are conserned that https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1085-L1106 will not be correct with this change
14:35:11 alex_xu yes, but that patch allow the overcommit for each numa cell, but in the later, we found it is cpu pinned instance, we only ensure it isn't overcommit by the amount memory for all the numa cells
14:35:57 alex_xu sean-k-mooney: no, I concern this branch https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1076-L1083...
14:36:26 sean-k-mooney ah well i just spotted that stpehn has not allowed oversubscition here also which shoudl be allowed https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L982-L990
14:37:05 sean-k-mooney or rather this should not be checking host_cell.avail_memory it should be checking host_cell.memory
14:37:22 sean-k-mooney or better it should be doing a page size aware check
14:37:26 stephenfin I don't think I changed that
14:37:41 sean-k-mooney stephenfin: you didnt that is my point
14:38:11 sean-k-mooney if you dont chenge that pinned instance can still not have over subsribed memory which is part of the original bug
14:38:34 stephenfin sean-k-mooney: Yes, that's what alex_xu is saying, I think :)
14:38:59 stephenfin However, I should point out that this is yet another corner case that I think I should fix in a follow-up
14:39:27 alex_xu sean-k-mooney: why that should check with host_cell.memory, sorry I didn't get the point
14:39:34 sean-k-mooney stephenfin: well alex_xu is also makeing that such a check shoudl be page size aware
14:40:00 sean-k-mooney alex_xu: checking avilable memory is checking free memory so it decresase with each vm booted
14:40:59 sean-k-mooney so if i have 20 gigabes of memory and ram allocation raitio of 1.5 avilable will alow only 2 10G vms to both and total will allow 3 respecting the allocation ratio
14:41:04 alex_xu sean-k-mooney: yea, that is we said not allow overcommit by checking free memory, right?
14:41:36 sean-k-mooney alex_xu: over commit of ram is intended to be support for pinned instances
14:41:53 alex_xu sean-k-mooney: ah...why
14:42:08 sean-k-mooney stephenfin: if you chage it to patial bug then it can be a follow up but if you want it to be closed bug then it needs to be in this change
14:42:28 alex_xu then I would say that isn't the problem of stephenfin's patch
14:42:42 openstackgerrit Matt Riedemann proposed openstack/nova stable/rocky: Restore nova-consoleauth to install docs https://review.openstack.org/630997
14:42:49 stephenfin sean-k-mooney: You sure? It's very much a different bug
14:42:50 sean-k-mooney alex_xu: the cpu pinning specs never forbid over subsription of memory and it was a bevhioral change that was intoduced in error after the fact with no spec
14:43:19 sean-k-mooney stephenfin: its not from my point of view its the exact same bug
14:43:23 alex_xu sean-k-mooney: ok, got it
14:44:09 alex_xu so that is another thing now
14:45:27 sean-k-mooney stephenfin: if you update the bug to specicialy state it dose not fix pinned instance and file a new bug then you could close the existing bug. i consider pinned instance to just be a special case of instnace with numa topologies
14:46:21 alex_xu whatever...close to 11pm, I need to go to bed now
14:47:56 sean-k-mooney alex_xu: good night o/
14:49:17 maciejjozefczyk stephenfin: hey, if you have a minute PTAL https://review.openstack.org/#/c/614167/ and https://review.openstack.org/#/c/591607. You have already reviewed that :) Thanks
14:50:14 stephenfin maciejjozefczyk: Just working through the above hugepage issues atm, but I'll stick it on my review queue :)
14:50:37 maciejjozefczyk stephenfin: good luck :)
14:53:33 sean-k-mooney stephenfin: ill reply to the bug and suggest coverign the pinning issue in a follwo up patch
14:53:47 sean-k-mooney as a seperate bug
14:56:41 stephenfin sean-k-mooney: I'm rather confused now http://paste.openstack.org/show/742644/
14:58:57 sean-k-mooney looking
15:00:37 sean-k-mooney host_cell.avail_memory include hugepages
15:01:08 sean-k-mooney so the first instace boots
15:01:19 sean-k-mooney and the second lets see
15:01:20 stephenfin yup, I get that. What's up with the second one though
15:01:31 stephenfin I figured this would prevent that from happening https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1021-L1036
15:02:05 ShilpaSD mriedem: Hi, replied to your comment and uploaded patch https://review.openstack.org/#/c/612626/2, Please give your feedback on the latest patch.
15:02:24 sean-k-mooney stephenfin: is this a dual numa system
15:02:52 stephenfin shouldn't matter. I've configured the same amount of hugepages for both, and both sockets appear to have ~ the same amount of memory
15:03:02 sean-k-mooney i see form the free you have 6.7G free but the cat command shows ~400MB free so im guessing its booting on numa node 1
15:03:32 sean-k-mooney stephenfin: you have configred 16GB of hugepages total
15:03:50 stephenfin sean-k-mooney: Yup, 8GB per node
15:04:02 sean-k-mooney so there are 8G of small pages on each node some of which are used by the os
15:04:26 sean-k-mooney oh its a 10G vm not 1G
15:04:31 stephenfin Yup
15:04:36 stephenfin Sorry, should have highlighted that
15:05:04 stephenfin 16 GB per node - 8 GB 2M hugepages should leave only 8 GB usable small pages per node
15:05:05 sean-k-mooney no i just missed the 0 at the end
15:05:13 sean-k-mooney yep
15:05:22 stephenfin So a 10 GB instance should fall flat on its face
15:05:48 sean-k-mooney so rather then looking at this output its better to look at libvirts
15:06:18 sean-k-mooney also what is your ram allocation ratio set to im assuming 1.5
15:06:40 stephenfin 16.0
15:06:42 stephenfin manually set
15:06:50 sean-k-mooney your ram is set to 16
15:06:50 stephenfin shouldn't matter for hugepages though
15:06:54 sean-k-mooney true
15:07:15 sean-k-mooney do you have your patch applied or master
15:07:31 stephenfin my patch
15:09:51 sean-k-mooney with an explicit pagesize of small we expect https://github.com/openstack/nova/blob/89c2d1056b3f37bad243c7468d974ded71336a0f/nova/virt/hardware.py#L623-L649 to reject it correct
15:10:52 stephenfin Yes, that would be my expectation
15:11:25 sean-k-mooney so you have not modifed that in your patch so that is either broken and has always been or there is something else going on
15:12:03 sean-k-mooney well you changed the name of the fuction it calls but otherwise its the same
15:27:10 openstackgerrit Matt Riedemann proposed openstack/nova master: The field instance_name was added to InstanceCreatePayload https://review.openstack.org/627967
15:36:12 openstackgerrit Jan Gutter proposed openstack/os-vif master: Clean up versioned object backlevelling code https://review.openstack.org/630976
15:40:14 sean-k-mooney stephenfin: looks like i was wrong about pinning and memory oversubsription. https://github.com/openstack/nova/commit/892e4032acea379bac6d1ae18fc5271ea17c0131 the intel implementation did not prevent it and the spec did not appove that behavior as a result we do not document it https://docs.openstack.org/nova/pike/admin/cpu-topologies.html#customizing-instance-cpu-pinning-policies. i consider that a
15:40:16 sean-k-mooney long term but which i think we should resolve seperatly
15:49:04 mdbooth cdent: Don't suppose you submitted that tox.ini patch, did you?
15:50:04 mdbooth cdent: NM, some gerrit-fu was a match to the task
15:52:27 mriedem lyarwood: is the conflict in here still valid? https://review.openstack.org/#/c/627958/
15:53:40 lyarwood mriedem: yeah it's actually due to I217fba9138132b107e9d62895d699d238392e761 landing in stable/rocky after the initial PS I posted
15:53:49 openstackgerrit Matt Riedemann proposed openstack/nova stable/rocky: libvirt: Add workaround to cleanup instance dir when using rbd https://review.openstack.org/627958
15:54:43 mriedem lyarwood: yeah fixe
15:54:45 mriedem *fixed
15:55:01 lyarwood huh, I don't get it anymore, weird.
15:55:19 lyarwood okay well it wasn't clean just after that landed but is now, meh!
16:20:09 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: docs: Secure live migration with QEMU-native TLS https://review.openstack.org/629627
16:20:10 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: A few miscellaneous items related to "native TLS" https://review.openstack.org/630980
16:20:44 kashyap (Okay, let's put the bikeshed paint back in the cellar.)
16:22:00 openstackgerrit Jan Gutter proposed openstack/os-vif master: Clean up versioned object backlevelling code https://review.openstack.org/630976
16:53:41 cdent mdbooth: did you find (and then try) that py37 tox fix? I was on the wrong machine to be able to give it a totally robust check
16:57:46 mdbooth cdent: Yes. Tested locally, all good.
16:57:54 cdent mdbooth: great, thanks
16:58:03 openstackgerrit Matt Riedemann proposed openstack/nova master: Send context.global_id on neutron calls https://review.openstack.org/627034
16:58:17 mdbooth cdent: I rebased my current work on top of it for convenience.
16:59:46 openstackgerrit Matt Riedemann proposed openstack/nova stable/rocky: Send context.global_id on neutron calls https://review.openstack.org/627603
17:01:02 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: Send context.global_id on neutron calls https://review.openstack.org/627604
17:03:08 mdbooth cdent: It breaks my 'testone' script, though, by not using the 'py37' env any more :/ I note that's by design, though.
17:03:30 cdent tis
17:06:11 cfriesen mriedem: so I guess the "specify timeout per live migration request" spec kind of died on the vine?

Earlier   Later