| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-07 | |||
| 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 | openstack | Launchpad bug 1785827 in OpenStack Compute (nova) "Performance regression in libvirt get_available_resource()" [Undecided,New] | |
| 14:06:52 | s10 | https://bugs.launchpad.net/nova/+bug/1785827 | |
| 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 | return | |
| 14:10:02 | mriedem | and self.updated > compute.updated_at): | |
| 14:10:02 | mriedem | if (self.updated and compute.updated_at | |
| 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 | mriedem | 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: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 | - Specify traits | |
| 14:21:14 | efried | - Specify a resource class | |
| 14:21:14 | efried | - Identify devices to be permitted for passthrough | |
| 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: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 | |
| 14:22:38 | sean-k-mooney | efried: um im not sure about that... given how we did the numa aware vswitch spec i would have assumed we would have used dynamic config insead of a yaml file | |
| 14:22:43 | efried | maciejjozefczyk: Thanks! | |
| 14:23:00 | efried | sean-k-mooney: -1 to dynamic config. | |
| 14:23:19 | kosamara | efried: I may propose something more basic, with the existing passthrough_whitelist conf. | |
| 14:23:50 | efried | sean-k-mooney: I think the only reason we did that instead of yaml is to minimize the effort. | |
| 14:23:55 | sean-k-mooney | kosamara: for network devices we use ethtool ioctls via libvirt to get the nic feature flags such as tcp checksume offload | |
| 14:23:56 | efried | kosamara: Oh dear gods please no. | |
| 14:24:35 | sean-k-mooney | kosamara: adding more stuff to passthough_whitelist is basically an automatic -3 | |
| 14:24:41 | mdbooth | lol | |
| 14:25:12 | efried | sean-k-mooney: We talked about using yaml in Denver. It really makes the most sense for this kind of thing, because trying to manage nested hierarchical data via oslo_config is a major PITA. | |
| 14:25:51 | sean-k-mooney | efried: well there is an argument to be made that today we dont use yaml for any other configs so we should not intoduce it for this feature | |
| 14:25:55 | efried | kosamara: ...Also, automatically generated traits. In my spec we've "namespaced" the generated traits with _POWERVM_ but some of them will potentially overlap on any platform (e.g. vendor & product IDs) | |
| 14:26:10 | sean-k-mooney | efried: that said im not really against it either | |
| 14:26:31 | kosamara | efried: I had it like that in my mind following a previous discussion with sean-k-mooney and gibi. I'll rethink it. | |
| 14:26:37 | efried | sean-k-mooney: IIRC jaypipes was a proponent and even dansmith was in agreement. | |
| 14:27:23 | efried | kosamara: In case the prospect of yaml schema/parsing is intimidating, here's code: https://review.openstack.org/#/c/579289/ | |
| 14:28:15 | sean-k-mooney | efried: well jaypipes didnt want more semantics to the whitelist and having a dedicated config not in nova was cleaner as nova was not using dynamic config at the time. as i said im not against it but its adding another dependicy to nova e.g. yaml parsing | |
| 14:29:01 | sean-k-mooney | efried: are you going to propose that spec to nova propper for stien? | |
| 14:29:58 | sean-k-mooney | efried: if so is the scope just pci devices or generic device passthouhg. we had talked about expanding it to usb/sata devices in denver too but not sure if that is a different spec | |
| 14:30:12 | sean-k-mooney | * should be a different sepc | |
| 14:31:05 | efried | sean-k-mooney: I hadn't yet decided whether to propose a nova spec for Stein or wait until T, but it sounds like kosamara may be interested in doing it for Stein. | |
| 14:31:33 | efried | sean-k-mooney: The way I've written the nova-powervm spec, the schema would be easily extensible to support non-PCI. | |
| 14:31:34 | jaypipes | efried: vendor and product IDs should not be traits. | |
| 14:31:47 | efried | sigh | |
| 14:31:58 | jaypipes | just sayin. | |
| 14:32:07 | mdbooth | lyarwood: Hey, this is interesting | |
| 14:32:40 | sean-k-mooney | efried: i abandonded my nic feature based schduling work in favour of using this in the furutre but not sure ill be working on that now | |
| 14:32:52 | mdbooth | lyarwood: Still investigating your functional failures, came across _terminate_volume_connections in ComputeManager, which does exactly what I proposed | |
| 14:33:03 | sean-k-mooney | jaypipes: im assuming you would prefer a resouce_class to track that | |