| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-01-15 | |||
| 14:12:05 | stephenfin | alex_xu: I saw the comment, but I'm not sure I understand the actual problem. What do you think could go wrong? | |
| 14:12:28 | stephenfin | Just saw you replied. Oops | |
| 14:13:11 | sean-k-mooney | instance_cell.memory is the amoung of guest ram | |
| 14:13:18 | stephenfin | alex_xu: So I think you're saying that https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L982 could be wrong? | |
| 14:13:20 | sean-k-mooney | on that virtual numa node | |
| 14:13:40 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: A few miscellaneous items related to "native TLS" https://review.openstack.org/630980 | |
| 14:13:52 | stephenfin | alex_xu: Or rather that 'host_cell.avail_memory' includes memory that's not actually available because it's used for hugepages | |
| 14:13:58 | sean-k-mooney | this will be compared to the amount of ram available for the page size selected | |
| 14:16:05 | stephenfin | alex_xu: Do I understand correctly? If so, I guess this would be easily checked. On a 64GB host, I create 34 1GB huge pages then try to boot a 32GB pinned instance. If it doesn't fail, we're likely overcommitting | |
| 14:16:26 | stephenfin | though if that is the case, it's a different bug again that I think we should probably handle in a follow-up | |
| 14:16:39 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: A few miscellaneous items related to "native TLS" https://review.openstack.org/630980 | |
| 14:17:20 | alex_xu | stephenfin: yes, that is the case I talk about | |
| 14:17:22 | sean-k-mooney | host_cell.available is compareing free memory not total but the page size check is done by https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1021-L1056 which is run after | |
| 14:17:47 | kashyap | Damn it, one last respin | |
| 14:17:52 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: A few miscellaneous items related to "native TLS" https://review.openstack.org/630980 | |
| 14:18:03 | kashyap | (Sorry for the spam, folks.) | |
| 14:18:22 | stephenfin | sean-k-mooney: Right, but I'm changing that here https://review.openstack.org/#/c/629281/1/nova/virt/hardware.py@1050 | |
| 14:18:31 | stephenfin | sean-k-mooney: to allow overcommit if using the smallest pagesize | |
| 14:19:15 | sean-k-mooney | yes i think it si still correct i like the wrong peice of code one sec | |
| 14:19:36 | stephenfin | sean-k-mooney: Ah, yes. https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L982 is available memory, as you say, not total memory | |
| 14:21:51 | stephenfin | but it's based on total memory (which could be include memory used for hugepages) minus memory consumed by instances | |
| 14:22:00 | stephenfin | ugh, I need to check this locally again | |
| 14:22:50 | sean-k-mooney | so you are changing _pack_instance_onto_cores to allow oversubsciption you are not modifying _numa_fit_instance_cell | |
| 14:24:22 | sean-k-mooney | _pack_instance_onto_cores is called from _numa_fit_instance_cell_with_pinning which is called in _numa_fit_instance_cell after the check on line 1046 | |
| 14:24:58 | sean-k-mooney | sorry no ignore that | |
| 14:25:21 | sean-k-mooney | gerrit is mseeing with me you are modifying _numa_fit_instance_cell | |
| 14:26:08 | stephenfin | sean-k-mooney: I'm modifying _numa_cell_supports_pagesize_request which is called by _numa_fit_instance_cell | |
| 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 | stephenfin | shouldn't matter for hugepages though | |
| 15:06:50 | sean-k-mooney | your ram is set to 16 | |
| 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 | |