| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-12 | |||
| 15:09:48 | cdent | sean-k-mooney: it depends on whether you are doing the loop the first time or subsequence times. 1st time is_bfv is the expensive part | |
| 15:10:16 | sean-k-mooney | i had expected self._update_usage_from_instances ot be more costly | |
| 15:10:32 | cdent | sean-k-mooney: yes, when bfv cache is cold, not but not otherwise | |
| 15:10:40 | mriedem | cdent: i'm assuming is_bfv is expensive since it has to make the is_volume_backed call which needs to get the bdms | |
| 15:10:46 | cdent | mriedem: yes | |
| 15:10:51 | cdent | and 1000 of those adds up | |
| 15:11:24 | mriedem | yeah...there is a change floating around where i asked someone if they could put a sqla joined field on the instances model to detect in a join query if an instance is volume-backed so we could optimize that | |
| 15:11:26 | mriedem | sec | |
| 15:11:50 | mriedem | the logic is pretty simple i think - is there a boot_index=0 bdm with destination_type=volume for this instance | |
| 15:11:52 | mriedem | if so, it's bfv | |
| 15:12:14 | cdent | once the cache is hot, it disappears as a problem which suggests that a similar instance cache would have similar properties... | |
| 15:12:26 | cdent | but managing such a cache is...tricky | |
| 15:12:30 | mriedem | well, my point is, | |
| 15:12:50 | cdent | but is exactly what I would think a "resource tracker" would do... | |
| 15:12:54 | mriedem | rather than (1) big InstanceList.get and then (2) big is_bfv cache build, if we could get the is_bfv value per instance in (1) then we avoid (2) | |
| 15:13:11 | mriedem | https://review.opendev.org/#/c/612626/10/nova/db/sqlalchemy/api.py | |
| 15:13:45 | cdent | sure, but that only helps on the first trip, right? | |
| 15:14:30 | efried | tssurya: I left a handful of comments as well | |
| 15:14:32 | cdent | btw: have you seen my comments about how the eventlet profiler gets all chundered up when RPC is in play? | |
| 15:14:48 | mriedem | no | |
| 15:14:53 | efried | cdent: just commented on that, I don't think n-cpu is supposed to talk to the db. | |
| 15:14:54 | cdent | turning off the indirection_api fixes it | |
| 15:15:10 | cdent | efried: only for hypervisor isolation, which is not a problem for vsphere... | |
| 15:15:32 | cdent | and for profiling/testing is not an issue | |
| 15:15:32 | mriedem | cdent: efried: it's not just isolation, it's also to support rolling upgrades | |
| 15:15:44 | cdent | which vsphere doesn't do either :) | |
| 15:15:46 | mriedem | temporarily disabling for profilng is one thing | |
| 15:16:03 | cdent | yeah, I wouldn't suggest there being some kind of config setting for it | |
| 15:16:05 | mriedem | we need a mogan for nova+ironic and a vogan for nova+vmware | |
| 15:16:10 | mriedem | and nova can then just be kvm like it already is | |
| 15:16:27 | cdent | i actually think that's probalby a good idea, sadly | |
| 15:16:48 | cdent | trying to be all things to many things is terrible | |
| 15:17:01 | mriedem | cdent: anyway, i'd probably investigate trying to detect races by comparing the instances you get from the db to the set of tracked_instances in the rt after you get the lock | |
| 15:17:11 | mriedem | and determine if you need to adjust your 'instances' like which might just be re-getting it | |
| 15:17:31 | mriedem | and then, like run rally against that to see if you can break it? idk. | |
| 15:17:44 | cdent | I'm still unclear on where a lost race experiences a problem. what breaks | |
| 15:18:23 | mriedem | claims related stuff, which at this point is numa/pci i think, | |
| 15:18:26 | sean-k-mooney | i think it woudl only be an issue for resouce that are not tracked in placmenet | |
| 15:18:29 | mriedem | since we just removed the cpu/ram/disk claims | |
| 15:20:05 | mriedem | tracked_migrations could also be wonky but on a smaller scale / tighter race | |
| 15:20:29 | cdent | go RT | |
| 15:21:13 | sean-k-mooney | well anythin relying on the perodic task to fix up allocation could be off but it should get fixed on the next run | |
| 15:21:27 | sean-k-mooney | missing frees is not that concuerning but missing claims would be | |
| 15:21:33 | cdent | sean-k-mooney: yeah, which ought to be fine | |
| 15:21:54 | mriedem | sean-k-mooney: the periodic doesn't fix up allocations... | |
| 15:21:59 | mriedem | not since ocata/pike anyway | |
| 15:22:36 | sean-k-mooney | out side of the migration ones? i though it healed allcoation for instace once the migration was conrimed | |
| 15:23:09 | mriedem | the rt does not do that | |
| 15:23:12 | cdent | sure it does: it spends lots of time in here: https://github.com/openstack/nova/blob/52b9359d6c6f387b8b9728c723c9a2501136d605/nova/compute/resource_tracker.py#L1314 | |
| 15:23:23 | cdent | orphans | |
| 15:23:23 | mriedem | the compute manager explicitly does stuff with migration-based allocations | |
| 15:23:54 | mriedem | cdent: that's about it since all of the other allocation "healing" code was removed | |
| 15:23:54 | cdent | ever single allocation for instance.uuid is compared to each instance on the host | |
| 15:24:13 | cdent | that chunk can get really loud becuase of https://github.com/openstack/nova/blob/52b9359d6c6f387b8b9728c723c9a2501136d605/nova/compute/resource_tracker.py#L1340 | |
| 15:24:22 | cdent | commenting out that line can save a bunch of log noise | |
| 15:24:51 | mriedem | debug("things are working normally") | |
| 15:24:55 | cdent | and 1372 there is one last remove | |
| 15:25:53 | sean-k-mooney | mriedem: i dont know im suspios that we normally would print that :P | |
| 15:26:14 | sean-k-mooney | but yes that does not really add much value | |
| 15:26:30 | cdent | if you both agree, I can put up a patch to remove it? | |
| 15:26:45 | cdent | meanwhile, I will create 1000 fake instances again | |
| 15:27:17 | mriedem | there used to be an audit level which is lower than debug, i'm not sure i'm totally in favor of removing it, | |
| 15:27:21 | mriedem | as the comment at the top says, | |
| 15:27:22 | mriedem | # NOTE(jaypipes): All of this code sucks. | |
| 15:27:34 | mriedem | but when i'm debugging rt issues, the more logging i have the better | |
| 15:27:39 | cdent | true | |
| 15:27:52 | mriedem | i frequently have to add logging to the rt when i'm debugging functional test fialures that involve it (like i was on friday for at least 2 hours) | |
| 15:27:59 | cdent | me too | |
| 15:28:11 | cdent | i have at least 6 additional LOG.debug in there right now | |
| 15:28:12 | sean-k-mooney | ya that is fair | |
| 15:28:32 | mriedem | https://review.opendev.org/#/c/366422/ | |
| 15:28:49 | sean-k-mooney | we do have a way to filter out log messages via config too right | |
| 15:28:50 | mriedem | i said AUDIT but i was thinking TRACE | |
| 15:29:40 | redkrieg | lyarwood: Are you around by chance? I found your blueprint for stable disk devices in rescue mode and it's exactly what I need to offer my customers reliable ISO support. Just wanted to know what the status is and whether there's anything I can work on related to it to get it in a release in the future: https://blueprints.launchpad.net/nova/+spec/virt-rescue-stable-disk-devices | |
| 15:30:42 | sean-k-mooney | just based on the ordering in the gate logs http://logs.openstack.org/38/671338/5/check/tempest-full-py3/12e1ea1/controller/logs/screen-n-cpu.txt.gz | |
| 15:31:00 | sean-k-mooney | i would have guess audit and trace were both higher then info/debug | |
| 15:31:30 | mriedem | i don't know how to filter specific messages from logging config | |
| 15:32:21 | sean-k-mooney | i think we can do it at the module level but i dont know if we can do it at the function level. we cant do it on indigiual message via config so it proably too course for this usecase | |
| 15:32:55 | sean-k-mooney | i.e. we dont want to jsut make the RT be info and everything else debug | |
| 15:33:57 | sean-k-mooney | i guess that message does serve a use in confiming which branch is taken | |
| 15:34:22 | sean-k-mooney | but for ops that run at debug level in production its proably noisier then they would like | |
| 15:36:22 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Avoid timeout from service update notification tests https://review.opendev.org/675935 | |
| 15:40:48 | openstack | Launchpad bug 1839853 in OpenStack Compute (nova) "Misuse of nova.objects.base.obj_equal_prims in tests" [Medium,Confirmed] | |
| 15:40:48 | mriedem | low-hanging-fruit test-only bug https://bugs.launchpad.net/nova/+bug/1839853 | |
| 15:41:07 | openstackgerrit | Jing Zhang proposed openstack/nova master: Remove the current host from cold migration target host list https://review.opendev.org/675025 | |
| 15:51:30 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Avoid timeout from service update notification tests https://review.opendev.org/675935 | |
| 15:56:40 | mriedem | cdent: up your vmware alley https://review.opendev.org/#/c/675025/2 | |
| 15:56:52 | cdent | sounds a bit rude, that | |
| 15:57:00 | mriedem | ha | |
| 15:58:06 | mriedem | dansmith: efried: melwitt: what do you think about the [workarounds]/allow_cold_migrate_to_same_host idea in https://review.opendev.org/#/c/675025/ as a backportable thing until we can be smarter about scheduling for cold migrate based on compute capabilities / traits filtering? | |
| 16:00:50 | dansmith | mriedem: meaning different behavior for cold migrate vs. resize? | |
| 16:01:11 | mriedem | yeah | |
| 16:01:35 | mriedem | the existing option conflates them which is bad for everything except vmware | |
| 16:01:41 | dansmith | the intent of cold migration is always to land it on a different host, so I guess I'm not sure why the patch is wrong (in intent, I haven't looked at the reasoning) | |
| 16:02:04 | mriedem | that's true for everything except vcenter | |
| 16:02:29 | dansmith | okay so the workaround config knob would only need tweaking for vmware? | |
| 16:02:32 | cdent | yet another raeson for vogan | |
| 16:02:42 | mriedem | dansmith: yeah | |
| 16:02:50 | melwitt | I'm not opposed to the patch but isn't this a super latent thing or? surprised this hasn't come up before | |
| 16:02:55 | mriedem | and is a backportable thing until we can be smarter about filtering based on traits | |