| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-04-28 | |||
| 13:13:08 | gibi | sean-k-mooney: I can reproduce the keyerror on master too with a bit of change and I think I see the issue. _create_tracker should take dict that come from the db with id field but _set_hvdevs should take dict that come from the hypervisor (no id field). My new tests (and one existing test) passed db dict to _set_hvdevs causing the db dict to get corrupted | |
| 13:13:52 | sean-k-mooney | ya so that can work if its a copy | |
| 13:13:52 | gibi | https://github.com/openstack/nova/blob/028b3bca16c750f6c7edf1b389ed6c79a2c9843d/nova/tests/unit/pci/test_manager.py#L361 | |
| 13:14:03 | sean-k-mooney | since it wont affect the gloabl sate but ya | |
| 13:14:25 | gibi | so we need the copy in L361 as that already corrupts the global state | |
| 13:14:34 | gibi | even on master | |
| 13:14:44 | sean-k-mooney | yep | |
| 13:15:02 | sean-k-mooney | well | |
| 13:15:11 | sean-k-mooney | we shoudl do the copy on line 347 | |
| 13:15:23 | sean-k-mooney | or we shoudl hvae _create_tracker do the copy internally | |
| 13:15:37 | sean-k-mooney | and then work on self.fake devs if need | |
| 13:16:24 | sean-k-mooney | but ya in anycase i wonder why we are not seeign this fail in general | |
| 13:16:38 | sean-k-mooney | theses test are not flaky in my experince | |
| 13:18:30 | gibi | we only have one test on master that does the mistake to pass a db dict with id to _sethvdevs. That corrupts the global state but no other test depends on that. Then I added another test that did this mistake and blow if the two test run in the same executor | |
| 13:18:41 | gibi | you can reproduce the issue by duplicating test_set_hvdev_remove_tree_maintained_with_allocations on master | |
| 13:18:49 | sean-k-mooney | ack ok | |
| 13:18:57 | gibi | I will try to clean this up | |
| 13:19:25 | sean-k-mooney | i wonder if we shoudl jsut have _create_tracker deep copy in genreal | |
| 13:20:03 | sean-k-mooney | we can still explictly do it but that would remove the need to do it in the default case | |
| 13:20:27 | sean-k-mooney | only if you call set_hvdevs | |
| 13:21:19 | sean-k-mooney | gibi: glad we caught that before it was merged | |
| 13:21:33 | sean-k-mooney | that kind of think is a pain to debug in the gate when it only failes ocationally | |
| 13:21:48 | gibi | yes, it was a good catch | |
| 13:21:59 | gibi | nobody likes interfeering test cases :) | |
| 13:56:01 | opendevreview | Balazs Gibizer proposed openstack/nova master: Isolate PCI tracker unit tests https://review.opendev.org/c/openstack/nova/+/839766 | |
| 13:56:20 | gibi | sean-k-mooney: ^^ I will base the current series on top of this | |
| 14:02:46 | opendevreview | Balazs Gibizer proposed openstack/nova master: Remove unavailable but not reported PCI devices at startup https://review.opendev.org/c/openstack/nova/+/838553 | |
| 14:02:46 | opendevreview | Balazs Gibizer proposed openstack/nova master: Isolate PCI tracker unit tests https://review.opendev.org/c/openstack/nova/+/839766 | |
| 14:02:47 | opendevreview | Balazs Gibizer proposed openstack/nova master: Allow claiming PCI PF if child VF is unavailable https://review.opendev.org/c/openstack/nova/+/838555 | |
| 14:02:47 | opendevreview | Balazs Gibizer proposed openstack/nova master: Simulate bug 1969496 https://review.opendev.org/c/openstack/nova/+/838554 | |
| 14:13:30 | sean-k-mooney | gibi: ack ok that makes sesne | |
| 15:21:08 | opendevreview | Elod Illes proposed openstack/nova master: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839781 | |
| 15:29:36 | elodilles | gibi bauzas : another stable gate fix to review ^^^ when you have time o:) | |
| 15:30:26 | elodilles | (today's broken oslo.policy release showed an error in our tox docs target as the job is failing for stable branches due to a release on zed) | |
| 15:46:05 | clarkb | elodilles: gibi bauzas email was sent about that problem a few weeks ago http://lists.openstack.org/pipermail/openstack-discuss/2022-April/028016.html there is a good chance that much of openstack needs that sort of update | |
| 15:47:04 | gibi | elodilles, clarkb: thanks I'm +2 on it | |
| 15:49:29 | sean-k-mooney | when did that get remvoed | |
| 15:49:42 | sean-k-mooney | we used to install requiremets.txt | |
| 15:50:27 | clarkb | sean-k-mooney: a while back there was a big push to switch to trimming the doc requirements down so you didn't have to install everything. What that missed was that the doc builds depended on the projects to collect cli command output and such. Basically I think it was docs having their own requirements that introduced the bug | |
| 15:50:40 | clarkb | the intent was good, but no one realized that this flaw existed | |
| 15:51:17 | sean-k-mooney | ah so it was applied genericly | |
| 15:51:26 | sean-k-mooney | i just did not recally this patch going in | |
| 15:51:55 | sean-k-mooney | we might also need test-requiremetns in some cases but in generaly not | |
| 15:53:54 | elodilles | clarkb: thanks, i'll try to check other projects as i've seen +24 broken stable-periodic tox-docs job today (neutron has already a similar patch on the gate right now) | |
| 15:54:13 | elodilles | (this one: https://review.opendev.org/c/openstack/neutron/+/839777 ) | |
| 15:59:01 | sean-k-mooney | clarkb: so it would b enice to have included the change id of the change that remvoed it but i dont think we shoudl hold this up for that so ill review it now | |
| 15:59:25 | clarkb | I mean its not my change. I just helped debug a similar problem a few weeks ago and we told everyone about it hoping they would audit and fix their repos | |
| 15:59:30 | clarkb | seems that didn't happen hence the current situation | |
| 15:59:50 | sean-k-mooney | i never new this happend i must have missed the mail | |
| 16:02:03 | clarkb | it was a huge cross openstack effort to change the doc build system | |
| 16:02:17 | clarkb | it was a while ago so I don't remember the details just that it happened and a lot of stuff got updates | |
| 17:57:25 | opendevreview | Merged openstack/nova master: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839781 | |
| 18:01:08 | opendevreview | Balazs Gibizer proposed openstack/nova stable/ussuri: Reproduce bug 1953359 https://review.opendev.org/c/openstack/nova/+/822047 | |
| 18:01:09 | opendevreview | Balazs Gibizer proposed openstack/nova stable/ussuri: Extend the reproducer for 1953359 and 1952915 https://review.opendev.org/c/openstack/nova/+/822048 | |
| 18:01:10 | opendevreview | Balazs Gibizer proposed openstack/nova stable/ussuri: [rt] Apply migration context for incoming migrations https://review.opendev.org/c/openstack/nova/+/822050 | |
| 18:17:25 | opendevreview | Elod Illes proposed openstack/nova stable/yoga: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839809 | |
| 18:22:21 | opendevreview | Elod Illes proposed openstack/nova stable/xena: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839810 | |
| 18:23:42 | opendevreview | Elod Illes proposed openstack/nova stable/wallaby: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839811 | |
| 18:25:32 | opendevreview | Elod Illes proposed openstack/nova stable/victoria: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839812 | |
| 18:26:42 | opendevreview | Elod Illes proposed openstack/nova stable/ussuri: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839813 | |
| 19:06:43 | melwitt | dansmith: yoga fix for docs job is ready https://review.opendev.org/c/openstack/nova/+/839809 | |
| 19:07:19 | dansmith | melwitt: I'm going to go out on a limb and say it'd be okay for you to slam those mofos in :) | |
| 19:07:40 | dansmith | you know, IMHO :D | |
| 19:08:06 | sean-k-mooney | i certenly would not object | |
| 19:11:15 | melwitt | haha ok | |
| 21:37:26 | opendevreview | Dan Smith proposed openstack/nova master: DNM: Run against performance.json patch https://review.opendev.org/c/openstack/nova/+/838934 | |
| 22:11:42 | dansmith | clarkb: around? | |
| 22:15:17 | clarkb | dansmith: hi | |
| 22:15:44 | dansmith | not even a full devstack run and 57k queries to the keystone db.. seems high, no? | |
| 22:16:16 | clarkb | dansmith: that does seem high. But openstackclient does have to get a new token for everything since there is no token caching | |
| 22:16:18 | clarkb | perhaps related to that? | |
| 22:16:30 | dansmith | still, 57k | |
| 22:16:39 | dansmith | also, they're almost all select | |
| 22:16:47 | dansmith | don't we have to insert when we create a token? | |
| 22:16:51 | clarkb | yes I think so | |
| 22:17:24 | dansmith | https://termbin.com/s2xj | |
| 22:17:26 | clarkb | I wonder if we need to instrument keystoen directly to try and identify that? | |
| 22:17:44 | dansmith | I think I'd like to know, cause that seems like some n^2 stuff to me | |
| 22:17:46 | clarkb | all of the other services look pretty reasonable | |
| 22:17:51 | dansmith | yes | |
| 22:17:56 | dansmith | neutron is pretty high, | |
| 22:18:14 | dansmith | and I see the number climb pretty fast when it's creating our network and subnet, which seems weird, | |
| 22:18:22 | dansmith | but it's still not 57k-level concerning | |
| 22:18:44 | clarkb | the token issuance would've been my first guess but I agree that those should be writes not reads | |
| 22:18:50 | dansmith | I would think | |
| 22:18:53 | clarkb | and even then we don't do 57k osc commands | |
| 22:18:55 | clarkb | like maybe 1k | |
| 22:18:57 | dansmith | right | |
| 22:19:30 | clarkb | I guess every other api request wiht a keystone token may have to validate with keystone? | |
| 22:19:44 | clarkb | but napkin math adding everything else together there doens't come close to 57k | |
| 22:20:13 | dansmith | yes, I expect validates to turn into selects, but still seems crazy high | |
| 22:20:47 | dansmith | 113 inserts, so maybe say 100 of those are tokens | |
| 22:21:09 | dansmith | that is 570 validates for each one | |
| 22:21:30 | dansmith | I guess catalog lookups maybe | |
| 22:21:48 | dansmith | but still, production systems must be getting _hammered) | |
| 22:21:58 | clarkb | it definitely seems like identifying the source of those and either reducingthem or making them more performant would be a worthwhile exercise | |
| 22:22:53 | dansmith | yeah curious to see if the keystone people think that's crazy or not | |
| 22:24:44 | dansmith | clarkb: check this: https://zuul.opendev.org/t/openstack/build/f31b8439b6dc47a19f9c99bbe3653d74/log/controller/logs/devstacklog.txt#20158 | |
| 22:24:49 | dansmith | 82k by the end of the devstack run | |
| 22:25:10 | dansmith | no wonder my 100k limit was rolling over on a full tempest run | |