| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-11 | |||
| 09:18:19 | kashyap | aspiers: NP; glad to be useful. Now I can bug you for Secure Boot stuff -- turning out to be more work than I anticipated :/ | |
| 09:18:27 | stephenfin | because we gave them no warning and no migration path in Stein | |
| 09:18:50 | stephenfin | alex_xu: However, yeah, I don't think this is in an issue. Consider this | |
| 09:19:58 | aspiers | kashyap: bug soon as I'm on vacation next week ;-) | |
| 09:20:01 | stephenfin | Stein-based compute node: reports VCPU, uses NUMATopology.cpuset to determine what cores can be pinned to (with the NUMATopologyFilter simply copying 'cpuset' to 'pcpuset' to keep that passing) | |
| 09:20:16 | kashyap | aspiers: Heh, noted; well-deserved | |
| 09:21:19 | stephenfin | Train-based compute node without new config options: reports VCPU, uses NUMATopology.pcpuset (p!) to determine cores can be pinned to but this == NUMATopology.cpuset (so no need for NUMATopologyFilter to copy 'cpuset' to 'pcpuset') | |
| 09:21:49 | alex_xu | yup | |
| 09:22:04 | stephenfin | Train-base compute node with new config options: reports PCPU, uses NUMATopology.pcpuset (p!) to determine cores can be pinned to and this is unique (again, no need for NUMATopologyFilter to copy 'cpuset' to 'pcpuset') | |
| 09:22:05 | stephenfin | sooo | |
| 09:22:23 | stephenfin | from a placement perspective, things have changed | |
| 09:23:00 | luyao | efried: do you need a unittest for this? https://review.opendev.org/#/c/678450/11/nova/compute/manager.py@2224 | |
| 09:23:10 | stephenfin | but from a NUMATopologyFilter (nova-scheduler) and nova-compute perspective, all three will work perfectly fine together | |
| 09:23:28 | alex_xu | yup, agree | |
| 09:24:01 | efried | luyao: was there a unit test for the exception case before? If so, it should be retained. If not, it wouldn't hurt to add one, but I'm not sure it's necessary. stephenfin, what do you think? | |
| 09:24:38 | luyao | efried: there was one before | |
| 09:24:50 | stephenfin | alex_xu: the _only_ thing that can hurt us is Train-based compute nodes that have no CPU configuration or have new config options but only for shared CPUs ('[compute] cpu_shared_set') | |
| 09:25:06 | efried | luyao: okay, so if it needs to be adjusted based on the code having been moved, do that, but don't remove the unit test. | |
| 09:25:22 | stephenfin | In that case, the second request to placement will return these hosts since they're reporting VCPU. However, they're _really_ reporting VCPU and don't have any PCPU inventory | |
| 09:25:27 | alex_xu | stephenfin: why | |
| 09:25:34 | stephenfin | but, like I said, the NUMATopologyFilter will protect us from that | |
| 09:26:05 | stephenfin | since NUMATopology.pcpuset will be set to None and therefore it will fail to pin | |
| 09:26:27 | luyao | efried: okay | |
| 09:26:56 | alex_xu | stephenfin: ok | |
| 09:26:58 | stephenfin | and obviously if NUMATopologyFilter is disabled for some reason, the instance will fail to schedule when it lands on the host | |
| 09:27:29 | alex_xu | stephenfin: that make senses I think | |
| 09:27:37 | alex_xu | stephenfin: have you see the resize case I wrote at https://review.opendev.org/#/c/681383/2 | |
| 09:27:41 | stephenfin | alex_xu: Phew, glad you understood. It's really hard to put this stuff into words without waffling or going down ratholes :-D | |
| 09:27:42 | stephenfin | Looking | |
| 09:27:53 | alex_xu | stephenfin: hah | |
| 09:28:40 | stephenfin | alex_xu: Could we call that out as a known issue? | |
| 09:29:22 | alex_xu | stephenfin: maybe, I'm thinking whether we request PCPU, and fallback to VCPU, can make that better. | |
| 09:29:30 | alex_xu | but I feel tired, my brain works very slow... | |
| 09:30:14 | stephenfin | Yeah, I guess we'd need to implement the retry logic at a higher level | |
| 09:30:29 | alex_xu | stephenfin: ah, I guess that doesn't work. The problem is placement always can get you same host allocation candidates, but the request is reject by the scheduler filters | |
| 09:31:00 | stephenfin | Yup. So if we wanted to solve that, we'd have to wrap both the call to placement and the call to the scheduler filters | |
| 09:31:15 | stephenfin | and the attempt to schedule to the guest too, in case the user doesn't have NUMATopologyFilter enabled | |
| 09:31:54 | stephenfin | that sounds like a lot of effort for corner case that will only happen while an operator is in the process of upgrading their configuration | |
| 09:33:20 | alex_xu | I feel I just tired | |
| 09:33:20 | stephenfin | alex_xu: If it gets too late for you, I can take over https://review.opendev.org/#/c/681383/ | |
| 09:34:02 | alex_xu | stephenfin: sorry, I need to take a rest little bit. you can free to take over those two patches. I probably online after one or two hours. | |
| 09:34:11 | stephenfin | Sounds good (y) | |
| 09:35:21 | efried | luyao: btw, here's what I did to that provider tree unit test when I was messing around with it locally: http://paste.openstack.org/show/775098/ | |
| 09:35:40 | efried | that last assertTrue fails with copy.copy but succeeds with deepcopy. | |
| 09:38:33 | luyao | efried: get it, thanks. | |
| 09:39:21 | efried | luyao: note that https://review.opendev.org/#/c/678453/26 is in merge conflict (probably aspiers' fault :P ) so you'll need to rebase again. | |
| 09:40:21 | luyao | efried: haha :D | |
| 09:49:43 | luyao | efried: I check the unittests for get_allocations, it tests the function that we move the get_allocations_xx out of | |
| 09:49:53 | luyao | efried: so I have to remove it | |
| 09:50:17 | efried | luyao: then I say just leave that part of the patch as is | |
| 09:50:42 | efried | luyao: stephenfin and I have already reviewed it and accepted it this way, so it's not worth the trouble of changing. | |
| 09:52:26 | luyao | efried: Okay | |
| 09:54:40 | luyao | efried: sorry, confirm again, need try/except or drop? | |
| 09:56:10 | efried | luyao: Sorry, this has gotten confusing. | |
| 09:56:10 | efried | Remove the try/except. I.e. keep this part of the patch as it appears in PS22 https://review.opendev.org/#/c/678450/22/nova/compute/manager.py@2215 | |
| 09:57:04 | luyao | efried: Okay, thanks. | |
| 09:58:56 | efried | luyao: Okay, I've finished re-reviewing the series (except for https://review.opendev.org/#/c/678455/, which I'm still not going to touch) | |
| 09:59:19 | efried | stephenfin: Do you need my re-look at any of the cpu-resources patches at this point? | |
| 09:59:29 | stephenfin | efried: In about a half hour, yeah | |
| 09:59:36 | stephenfin | I think the quota problem is solved | |
| 09:59:53 | efried | sweet | |
| 09:59:56 | stephenfin | So it's just replacing the scheduler conf option with retry logic for placement left | |
| 10:00:02 | stephenfin | almost done with tests | |
| 10:03:35 | efried | luyao: note response on the defaultdict thing -- I still don't think you need it https://review.opendev.org/#/c/678449/21/nova/compute/provider_tree.py@70 | |
| 10:06:58 | luyao | efried: you're right. :) | |
| 10:07:13 | efried | phew. I like being right | |
| 10:08:26 | bauzas | stephenfin: I wonder something | |
| 10:08:54 | bauzas | stephenfin: if we ask for an o.vo field not existing, can we just do something like 'if field not in my_object:' ? | |
| 10:09:07 | bauzas | IIRC, we need to use a specific o.vo method | |
| 10:09:13 | bauzas | unless the field is set to None | |
| 10:09:14 | stephenfin | bauzas: You can do that, yeah | |
| 10:09:17 | stephenfin | For some time now | |
| 10:09:33 | stephenfin | you used to need to use is_attr_set or something like that | |
| 10:09:37 | bauzas | ok, I had concerns about the virt.hardware module in https://review.opendev.org/#/c/671800/ | |
| 10:09:44 | stephenfin | but __contains__ was implemented many moons ago | |
| 10:09:59 | stephenfin | anywhere is particular? | |
| 10:17:39 | bauzas | nah it's okay then | |
| 10:18:02 | bauzas | my o.vo skills are a bit rusty | |
| 10:18:07 | bauzas | but I could have tested it | |
| 10:19:04 | sean-k-mooney | efried: after 6 year of working on nova the main lesson i have learns is unlesss you have 2 cores from different compaines lined up to review the think you want to do before you start it will take 2 cycles to complete | |
| 10:19:39 | sean-k-mooney | the first cycle you get it working and people beging to become familar with and the second you finall land the thing | |
| 10:20:22 | efried | sean-k-mooney: I would love to be able to commit cores to review blueprints before/as they are approved. I just a) doubt cores would be willing to commit, and b) don't want to do that much PM type work | |
| 10:21:43 | sean-k-mooney | its partly a social thing. you need to explain why this thing you care about is imporant so that people will be interested enogh to review and demonstrate it works and is sane. | |
| 10:22:21 | sean-k-mooney | the non craze telco things i have worked on are less hard to enabel but im normlally in craze telco land | |
| 10:22:26 | efried | It sucks that "be interested" is a criterion | |
| 10:23:31 | sean-k-mooney | well it only is in terms of making it higher on list of other things you have to review | |
| 10:24:03 | sean-k-mooney | there are only a limit number of sme fore each system and they are overloaded frequently | |
| 10:25:14 | sean-k-mooney | if we had more subsystem maintainer or more reviewres it would help but we work with what we got. | |
| 10:25:31 | sean-k-mooney | we still get a lot done each cycle | |
| 10:25:42 | openstackgerrit | Stephen Finucane proposed openstack/nova master: DNM: Try to fallback to PCPU request when VCPU failed https://review.opendev.org/681383 | |
| 10:25:42 | sean-k-mooney | the latency can just be a bit high sometimes | |
| 10:25:56 | stephenfin | efried, bauzas: Can you sanity check before I waste more time fixing up tests ^ | |
| 10:26:06 | stephenfin | the approach in general, that is | |
| 10:26:42 | bauzas | stephenfin: I left a comment on https://review.opendev.org/#/c/671800/ | |
| 10:28:03 | bauzas | stephenfin: cool, I'll do it a bit later (needs to refuel my stomach) | |
| 10:32:46 | efried | stephenfin: sorry, looking at this patch for the first time, and... is this really the right approach? If we bounce with PCPUs, try VCPUs?? If they *have* cut over, and we're just *actually* out of PCPUs in the cloud, they're going to get *not* desired behavior, nah? | |
| 10:33:21 | openstackgerrit | Luyao Zhong proposed openstack/nova master: db: Add resources column in instance_extra table https://review.opendev.org/678447 | |
| 10:33:21 | openstackgerrit | Luyao Zhong proposed openstack/nova master: object: Introduce Resource and ResourceList objs https://review.opendev.org/678448 | |
| 10:33:22 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Add resources dict into _Provider https://review.opendev.org/678449 | |
| 10:33:22 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Retrieve the allocations early https://review.opendev.org/678450 | |