| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-07 | |||
| 13:26:20 | kosamara | efried: yes. I'm working at CERN and I think you had a relevant discussion with Belmiro late June. | |
| 13:27:18 | efried | kosamara: I would be happy to talk through it further. I assume you're interested in doing this with libvirt, or do you have Power systems in your deployment? | |
| 13:27:45 | kosamara | efried: We would like at least a simplified version of what you're developing. With the prospect of working on it, I was looking for any relevant specs, but only came across yours recently. | |
| 13:27:59 | kosamara | efried: libvirt | |
| 13:28:13 | efried | kosamara: Are you aware of the cyborg project? | |
| 13:29:19 | kosamara | efried: yes, but it appears to be doing much more than what we need, so a solution within nova to use GPUs seems better at this point. | |
| 13:30:02 | efried | kosamara: How much have you done with the existing PCI passthrough framework? | |
| 13:30:51 | kosamara | efried: we are already using it in testing and slowly moving to production. | |
| 13:32:16 | kosamara | efried: and only for GPUs | |
| 13:32:18 | s10 | Hello. I've found, that function update_available_resource(), https://github.com/openstack/nova/blob/stable/pike/nova/compute/resource_tracker.py#L694 , driver.get_available_resource(nodename) with Libvirt driver takes 30 seconds to execute on the host with 150 instances on local storage. | |
| 13:32:18 | s10 | Is this behaviour normal or is it a regression introduced by https://github.com/openstack/nova/commit/d88b75e81eabfbd463007f6a4f27e6966a466530 and following commits? Before this commit it was 1-2 seconds for all of them. | |
| 13:32:20 | efried | kosamara: Okay. That's going to be your best bet for the near future. And it should do pretty much everything you need if all you're trying to do is pass through whole GPUs. | |
| 13:33:47 | efried | s10: You've specifically nailed it down to that commit? If you revert it, your performance goes back to normal? | |
| 13:34:41 | kosamara | efried: Whole GPUs is our main use case ATM. "that"? Our main problems with the current pci passthrough in nova are quotas and scheduling without an extra filter. So basically, implementing RPs/RCs for GPUs. | |
| 13:35:59 | s10 | Yes, if I return this function to be like before this commit, performance of this function goes back to normal. But then we will lose all fixes, introduced by this commits and following, like https://github.com/openstack/nova/commit/938c0a745325fa73d098c6d5ddd20b2a599f9624 | |
| 13:37:20 | s10 | efried: if i turn on logging for oslo_concurrency, I see, that a lot of time takes for 300 calls of "qemu-img info" for /var/lib/nova/instances/UUID/disk and disk.config | |
| 13:38:18 | s10 | efried: at least 0.052 for every call. 300 calls - 15 seconds. | |
| 13:40:38 | efried | kosamara: Not sure if we've really started making plans to implement quotas around placement artifacts yet. alex_xu, were you working on that? | |
| 13:41:49 | efried | s10: Let me take a look at what's piled on top of that commit. Trying to figure out what the effect would be of *just* reverting that one fix. | |
| 13:41:50 | s10 | efried: so for 150 instances qemu_img info is being called 600 times. 2 times for dk_size = disk_api.get_allocated_disk_size(path) and 2 times for virt_size = disk_api.get_disk_size(path). | |
| 13:42:12 | dansmith | s10: maybe talk to lyarwood about it | |
| 13:42:39 | efried | s10: Note that we have some work ongoing to cut that in half at least: https://review.openstack.org/#/c/520024/ | |
| 13:43:15 | efried | s10: But that still wouldn't make it okay as performance regressions go. | |
| 13:43:34 | sean-k-mooney | s10: we could proably cache it but is it an apricalble portion of the total boot time | |
| 13:45:20 | sean-k-mooney | there are other cases where we could chache thing in the boot process but dont because it was felt the caching introduced complexity that did not result in a significant perfomace increase when taken as a propotion of the total boot time | |
| 13:46:46 | s10 | efried: I added timer before driver.get_available_resource(nodename) and stopped it after and logged it, so only one call of this function takes 30 seconds. | |
| 13:46:56 | efried | s10: I think your best course of action right now is to open a bug and make sure lyarwood sees it. | |
| 13:47:24 | efried | s10: Do you have the ability to apply a single patch on the fly in your env and reproduce the flow? | |
| 13:47:43 | s10 | efried: yes, I have such ability | |
| 13:48:06 | efried | s10: I can try throwing out a quick revert just to confirm that it does the trick. Stand by. | |
| 13:51:40 | openstackgerrit | Eric Fried proposed openstack/nova stable/pike: DNM: Revert d88b75e https://review.openstack.org/589479 | |
| 13:51:46 | efried | s10: ^ | |
| 13:52:30 | lyarwood | s10: and this is with an images_type of raw? | |
| 13:52:32 | efried | s10: Note: a real revert would be tied to a bug and introduced in master and backported. This is just to confirm. | |
| 13:53:03 | kosamara | efried: alex_xu had proposed a spec for that. The first step is to have pci passthrough GPUs in placement of course. | |
| 13:53:09 | mriedem | yay https://review.openstack.org/#/q/If642e51a4e186833349a8e30b04224a3687f5594 | |
| 13:53:11 | s10 | efried: Before: Took 20.89 seconds to get available resources for nodename. update_available_resource /usr/lib/python2.7/dist-packages/nova/compute/resource_tracker.py:705 | |
| 13:53:29 | s10 | efried: After: Took 11.04 seconds to get available resources for nodename | |
| 13:53:45 | sean-k-mooney | kosamara: the first step to having pcie gpus in placement is having pci devices in placement | |
| 13:53:51 | s10 | If I change virt_size = disk_api.get_disk_size(path) same way, this time reduces to 2 seconds. | |
| 13:54:28 | efried | Okay s10, I'll leave you in lyarwood's capable hands at this point. | |
| 13:54:52 | s10 | lyarwood: yes, image_type is raw | |
| 13:55:08 | mriedem | it's going to be slower b/c it's exec'ing qemu-img info | |
| 13:55:10 | lyarwood | s10: unfortunatley I'm just between calls, could you write this up in a bug and I'll get back to you in ~60mins or so | |
| 13:55:32 | s10 | lyarwood: and preallocate_images=space. I will fill bug report. | |
| 13:58:08 | mriedem | dansmith: want to send https://review.openstack.org/#/q/topic:bug/1784705+(status:open+OR+status:merged)+branch:stable/queens to their maker? | |
| 13:58:55 | dansmith | mriedem: yeah | |
| 14:00:07 | maciejjozefczyk | mriedem: efried hey, about https://review.openstack.org/#/c/520024; we have it on production and it works properly; release is newton | |
| 14:00:52 | efried | maciejjozefczyk: Sweet, thanks for the info. mriedem cdent Ima +2 that sucker. Shall we backport it too? | |
| 14:01:06 | maciejjozefczyk | only one thing is that, as I remember correctly, on nova master was issue with https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L866 def_resource_change() | |
| 14:01:23 | maciejjozefczyk | efried: so could you please take me a moment to confirm that its fiexed on master, or not? | |
| 14:01:58 | cdent | efried: assume maciejjozefczyk's concerns there are okay, I think a backport would be nice but not critical? | |
| 14:02:53 | mriedem | maciejjozefczyk: you said you think https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L866 should be removed yes? | |
| 14:03:42 | maciejjozefczyk | mriedem: afailr I know it says all the time 'False'; so the update is not send to the DB | |
| 14:03:54 | maciejjozefczyk | and it ends with old timestamp for compute row | |
| 14:04:04 | maciejjozefczyk | so it could be deleted | |
| 14:04:34 | maciejjozefczyk | It was running all the time because in memory some resources were changed (because of this double-update) so it was always returning 'True' | |
| 14:04:54 | mriedem | is it always False because we've already updated the compute node before we get here? | |
| 14:06:15 | maciejjozefczyk | nope, if you apply my patch - compute is def _update() is called only once. but inside def _update() there is a check if somethings change about ram,disk etc in memory | |
| 14:06:31 | maciejjozefczyk | so without any change - it ends with False and the DB update is not called | |
| 14:06:52 | s10 | https://bugs.launchpad.net/nova/+bug/1785827 | |
| 14:06:52 | openstack | Launchpad bug 1785827 in OpenStack Compute (nova) "Performance regression in libvirt get_available_resource()" [Undecided,New] | |
| 14:07:01 | mriedem | is there any reason to update the compute node record in the db if nothing changed? | |
| 14:07:25 | s10 | lyarwood: ^ i've written a bug report | |
| 14:07:34 | maciejjozefczyk | without my change - it was called basically all the time, because first update was without for ex: shutdown instances, and the second was with (so the amout of ram, disk, etc was always changing - the def _resource_change() was saying 'True') | |
| 14:07:41 | lyarwood | s10: ack thanks | |
| 14:07:47 | maciejjozefczyk | mriedem: exept timestamp, no | |
| 14:07:49 | maciejjozefczyk | except* | |
| 14:07:51 | kosamara | sean-k-mooney: yes, which is what efried 's spec does. | |
| 14:08:30 | maciejjozefczyk | mriedem: but for me if 'nova compute-show' shows that compute is UP and timestamp is new - it says, yea, compute is working | |
| 14:08:51 | maciejjozefczyk | mriedem: maybe somebody use it as source for some scripts | |
| 14:09:01 | maciejjozefczyk | mriedem: but ye, service-list should be used anyway for that pourpose | |
| 14:09:19 | mriedem | i want to say i think there is something in the scheduler that cares about the compute node updated_at time and uses it for some refresh threshold | |
| 14:09:43 | maciejjozefczyk | mriedem: no idea | |
| 14:09:57 | mriedem | HostManager._check_for_nodes_rebalance | |
| 14:10:02 | mriedem | if (self.updated and compute.updated_at | |
| 14:10:02 | mriedem | and self.updated > compute.updated_at): | |
| 14:10:02 | mriedem | return | |
| 14:10:56 | maciejjozefczyk | yep, so thats the issue | |
| 14:11:31 | maciejjozefczyk | mriedem: the code is from placement? | |
| 14:11:40 | mriedem | no that's in the nova scheduler HostManager | |
| 14:11:53 | maciejjozefczyk | mriedem: https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L929 | |
| 14:12:03 | sean-k-mooney | kosamara: oh i did not know efried had a pci device in placement spec up for stien cool il should review that | |
| 14:12:18 | maciejjozefczyk | mriedem: ah, so it bases on the db, right? | |
| 14:12:21 | kosamara | efried: I would like to contribute. I think I could start with making the libvirt driver report PCI RPs, similar to your spec. I can propose a small spec just for that for Stein. | |
| 14:12:21 | mriedem | right | |
| 14:12:30 | mriedem | https://github.com/openstack/nova/blob/536acbfe0572f10ea84f330f2f29b07ca9279114/nova/scheduler/host_manager.py#L194 | |
| 14:12:55 | kosamara | sean-k-mooney: not in nova, in the nova-powervm fork: https://review.openstack.org/#/c/579359/10/doc/source/specs/rocky/device-passthrough.rst | |
| 14:13:13 | maciejjozefczyk | ok, so I vote for dropping resource_change logic, but first let me see how we use it in our deployment | |
| 14:13:17 | efried | kosamara: That would be cool, please add me as a reviewer. | |
| 14:13:22 | sean-k-mooney | kosamara: if your are doing that can you also make sure to pull the pci feature flags from the pci_devices table in the nova db and add them as traits to the RP | |
| 14:13:52 | maciejjozefczyk | mriedem: anyway my RUN team doesn't scream about this issue anymore | |
| 14:13:55 | mriedem | maciejjozefczyk: since I40c17ed88f50ecbdedc4daf368fff10e90e7be11 i'm not sure this check in the HostState object even matters | |
| 14:14:06 | mriedem | we don't cache HostStates in the scheduler anymore | |
| 14:21:14 | efried | kosamara: One main aspect of the nova-powervm spec that I expect to be carried through to nova proper is a YAML configuration file allowing the operator to: | |
| 14:21:14 | efried | - Identify devices to be permitted for passthrough | |
| 14:21:14 | efried | - Specify a resource class | |
| 14:21:14 | efried | - Specify traits | |
| 14:22:13 | kosamara | sean-k-mooney: I wasn't aware of this info in the pci_devices table. What exactly is it? | |
| 14:22:33 | maciejjozefczyk | mriedem: anyway I need to go, I'll check this resource_updated() logic with my patch once again, I'll leave comment tonight | |