| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-12-06 | |||
| 15:44:45 | dansmith | yeah | |
| 15:44:51 | efried_cya_jan | uh oh | |
| 15:45:17 | dansmith | mriedem: is there a warn-once pattern I should be using for logs? | |
| 15:46:23 | mriedem | dansmith: i think just a global | |
| 15:46:31 | dansmith | ack | |
| 15:46:48 | mriedem | i'll strip out a separate bug for this report client init thing | |
| 15:51:01 | mriedem | https://bugs.launchpad.net/nova/+bug/1807219 | |
| 15:51:01 | openstack | Launchpad bug 1807219 in OpenStack Compute (nova) "SchedulerReporClient init slows down nova-api startup" [Medium,Triaged] | |
| 15:52:22 | cdent | mdbooth: is that ^ related at all to the slow down you were seeing in your explorations yesterday(?)? | |
| 15:53:20 | efried_cya_jan | mriedem: We ought to singleton that guy. If we're not having caching conflicts, it can only be out of luck because the API obj isn't doing anything that touches the cache. | |
| 15:53:47 | edleafe | efried_cya_jan: more like efried_big_fat_liar | |
| 15:54:45 | efried_cya_jan | I'm here for like another 20 minutes today. This does two things: 1) keeps my inbox down to triple digits; 2) makes you scared to talk about me behind my back. | |
| 15:55:03 | mriedem | i intentionally talked about you to summon you | |
| 15:55:06 | mdbooth | cdent: Only if we also do this during server create. Possible. The number of API objects created was insane. | |
| 15:55:08 | kaisers_ | stephenfin: mdbooth: Updated https://review.openstack.org/#/c/554195/ as discussed yesterday | |
| 15:55:15 | edleafe | Oh, it's more fun to talk about you to your face! | |
| 15:56:23 | efried_cya_jan | mriedem: Is the removal of lazy-load causing (or even related to) that one-reportclient-per-API? | |
| 15:57:59 | mriedem | efried_cya_jan: not sure, we might have always been loading it during API init | |
| 15:58:41 | mriedem | but i think it would have done the lazy-load | |
| 15:58:52 | mriedem | since LazyLoader only creates the client thing until something is accessed on it | |
| 15:59:10 | efried_cya_jan | And it's only expensive because it's serialized, not because the report client is doing anything heavy, right? | |
| 15:59:29 | mriedem | yeah i think so | |
| 16:00:00 | mriedem | https://github.com/openstack/nova/blob/c9dca64fa64005e5bea327f06a7a3f4821ab72b1/nova/scheduler/client/report.py#L260 | |
| 16:00:21 | mriedem | we create 2 provider trees in there for some reason... | |
| 16:00:26 | mriedem | https://github.com/openstack/nova/blob/c9dca64fa64005e5bea327f06a7a3f4821ab72b1/nova/scheduler/client/report.py#L270 | |
| 16:00:31 | mriedem | https://github.com/openstack/nova/blob/c9dca64fa64005e5bea327f06a7a3f4821ab72b1/nova/scheduler/client/report.py#L279 | |
| 16:00:35 | mriedem | oops | |
| 16:00:40 | mriedem | https://github.com/openstack/nova/blob/c9dca64fa64005e5bea327f06a7a3f4821ab72b1/nova/scheduler/client/report.py#L286 | |
| 16:01:15 | mriedem | is there any good reason we need to do that in both places? | |
| 16:02:33 | efried_cya_jan | mriedem: There should never be a need for one process to create two separate provider trees <=> report client instances, period. | |
| 16:02:48 | efried_cya_jan | That could only ever lead to bugs. | |
| 16:02:57 | efried_cya_jan | It will only ever *not* lead to bugs by luck. | |
| 16:03:12 | efried_cya_jan | hold on, looking at your links... | |
| 16:03:24 | mriedem | so, i'm going to remove this https://github.com/openstack/nova/blob/c9dca64fa64005e5bea327f06a7a3f4821ab72b1/nova/scheduler/client/report.py#L286-L287 | |
| 16:03:35 | mriedem | and change https://github.com/openstack/nova/blob/c9dca64fa64005e5bea327f06a7a3f4821ab72b1/nova/scheduler/client/report.py#L270-L272 | |
| 16:03:39 | mriedem | to just call clear_provider_cache | |
| 16:03:45 | mriedem | for great success | |
| 16:03:47 | mriedem | ok?! | |
| 16:03:56 | mriedem | butthole surfers got me all amped up | |
| 16:05:18 | efried_cya_jan | mriedem: That will be fine, it's removing redundant calls, but it's not going to change anything functionally. We were calling ProviderTree init twice before, but not creating two separate provider trees, just overwriting self._provider_tree. | |
| 16:06:06 | efried_cya_jan | mriedem: However, I don't agree with the details. You can't get rid of _create_client without affecting @safe_connect. For now (until @safe_connect has died a fiery death), I would just remove the provider tree and assoc timer inits from __init__. | |
| 16:06:39 | efried_cya_jan | And if you would like to take a step on the road to that fiery demise of @safe_connect, you could review https://review.openstack.org/#/c/613613/ :P | |
| 16:07:52 | mriedem | efried_cya_jan: safe_connect doesn't care about provider tree and association refresh | |
| 16:08:01 | mriedem | only the ksa client thing | |
| 16:08:10 | melwitt | o/ | |
| 16:08:49 | efried_cya_jan | mriedem: reason we want to clear the cache in safe_connect is because it happened due to an error, so we can't count on our cache being correct. | |
| 16:09:29 | efried_cya_jan | I sense you're trying to find a way to remove the use of that semaphore. I don't think we can do that. | |
| 16:09:36 | mriedem | i'm not | |
| 16:09:48 | mriedem | i'm just trying to get rid of this code creating the provider tree data structure 20 times in different places | |
| 16:09:51 | mriedem | because i'm OCD | |
| 16:10:00 | mriedem | but i hear you | |
| 16:10:20 | efried_cya_jan | What you can do is make _create_client call clear_provider_cache, and remove those inits from __init__. That ought to consolidate the actual LOCs that do the init to one place. | |
| 16:10:26 | efried_cya_jan | I think. | |
| 16:10:51 | efried_cya_jan | but hm, this makes me wonder whether clear_provider_cache needs to be under that same lock. | |
| 16:10:59 | efried_cya_jan | (for the other cases where it's called) | |
| 16:11:16 | efried_cya_jan | I'll leave that steaming pile in front of you and walk away. | |
| 16:15:36 | efried_cya_jan | kashyap: Yeah, right about... now. | |
| 16:15:37 | efried_cya_jan | o/ | |
| 16:15:45 | kashyap | (That's good; happy to see screen-starers getting breaks...) | |
| 16:15:54 | sean-k-mooney | efried_cya_jan: enjoy the break | |
| 16:15:58 | kashyap | efried_cya_jan: Glad; don't make the mistake of staying connected to the VPN :D | |
| 16:29:23 | mriedem | heh putting the same lock on clear_provider_cache makes the tests lock up | |
| 16:32:53 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Only construct SchedulerReportClient on first access from API https://review.openstack.org/623246 | |
| 16:32:53 | openstackgerrit | Matt Riedemann proposed openstack/nova master: DRY up SchedulerReportClient init https://review.openstack.org/623247 | |
| 16:33:58 | mriedem | someone should probably tell the intel nfv ci to stop reporting b/c it's busted | |
| 16:34:01 | mriedem | and has been for awhile | |
| 16:40:58 | mnaser | https://wiki.openstack.org/wiki/ThirdPartySystems/Intel_NFV_CI | |
| 16:41:08 | mnaser | looks like it's in wip or something | |
| 16:43:52 | gibi | mriedem: my organized thoughts about the legacy notification deprecation http://lists.openstack.org/pipermail/openstack-discuss/2018-December/000685.html | |
| 16:45:12 | pas-ha | hi all, have a question re nova + barbican integration - if we have enabled `[glance]verify_glance_signatures` in nova, how booting from snapshot (including unshelve of instance) is supposed to work? AFAIU nova does not re-sign the snapshots it creates... | |
| 16:48:15 | mriedem | pas-ha: unfortunately everyone that i know that worked on that is no longer around, but people in #openstack-barbican might know | |
| 16:48:43 | mriedem | gibi: thanks | |
| 16:48:53 | pas-ha | mriedem: thanks, will ask around there then :-) | |
| 16:49:04 | gibi | mriedem: it might not help much but at least it summarizes our options | |
| 16:49:17 | mriedem | gibi: yeah that's still useful | |
| 16:50:53 | mriedem | pas-ha: i assume you mean because of this https://docs.openstack.org/nova/rocky/configuration/config.html#DEFAULT.non_inheritable_image_properties | |
| 16:51:02 | mriedem | so when nova creates a snapshot, the image snapshot does not inherit the signature | |
| 16:51:11 | mriedem | so trying to boot/unshelve from it later won't work | |
| 16:51:22 | mriedem | if you have verify_glance_signatures=True | |
| 16:52:21 | mriedem | interestingly enough, but sadly probably not surprising, is that those image properties are not documented https://docs.openstack.org/glance/latest/admin/useful-image-properties.html | |
| 16:52:57 | pas-ha | yep, but AFAIU even if they were passed, the image signature is no longer valid already as surely the hash of the snapshot is not the same as that of pristine image | |
| 16:53:08 | mriedem | yeah | |
| 16:53:10 | mriedem | i guess there is https://docs.openstack.org/glance/latest/user/signature.html | |
| 16:53:56 | mriedem | i guess the user would need to sign the snapshots after nova creates them right? | |
| 16:56:32 | pas-ha | right now that's the only way I see it might work, yes | |
| 16:57:29 | mriedem | this was the spec if that contains any info https://specs.openstack.org/openstack/nova-specs/specs/mitaka/implemented/image-verification.html | |
| 16:58:38 | mriedem | it does say "Add functionality to Nova which calls the standalone module when Nova uploads a Glance image and the verify_glance_signatures configuration flag is set." | |
| 16:59:23 | mriedem | but i don't actually see that happening | |
| 17:00:07 | mriedem | pas-ha: you might be better off simply asking about this in the openstack-discuss mailing list to see if anyone else is working on this already | |
| 17:00:10 | mriedem | or interested in closing this gap | |
| 17:00:30 | pas-ha | yes, figured that out, will do, thanks! | |
| 17:00:35 | mriedem | yw | |
| 17:54:11 | cfriesen | there's contact information there...is someone going to email them? | |
| 17:54:26 | cfriesen | ^ for the intel NFV CI | |
| 17:54:46 | sean-k-mooney | is it broken? | |
| 17:55:02 | cfriesen | mriedem: says yes | |
| 17:55:25 | cfriesen | bah, shouldn't have colon there | |
| 17:55:38 | sean-k-mooney | i read it anyway :) | |
| 17:56:01 | sean-k-mooney | i need to talk to infra and see if we can migrate that ci upstream | |
| 17:56:21 | sean-k-mooney | it all comes down to having host noes with nested virt | |