| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-26 | |||
| 02:42:18 | melwitt | I don't see anywhere in _load_instance that it sets instance._context | |
| 02:42:42 | melwitt | self.instance._context | |
| 02:43:15 | dansmith | melwitt: you're right, I remember arguing with alaski about this specifically | |
| 02:43:29 | dansmith | melwitt: so what do you want me to change it to? | |
| 02:43:31 | melwitt | yeah, I was not sure if it's intentional not to do it | |
| 02:43:42 | dansmith | I wanted him to, and he wanted it to be orphaned | |
| 02:44:02 | melwitt | hrm. | |
| 02:44:41 | melwitt | assuming he had good reason for that (we should ask him next time we catch him in the channel), I would think to go back to what you had, sans the refresh() call | |
| 02:45:04 | dansmith | but your comment above the refresh explains why that's important | |
| 02:46:51 | melwitt | why what's important? | |
| 02:47:05 | dansmith | we also have to handle the case where we didn't actually look it up right? | |
| 02:47:11 | dansmith | why the refresh is important | |
| 02:47:24 | dansmith | or you want to keep the instance we got from _lookup? | |
| 02:47:57 | melwitt | oh, I was saying that's another more complicated option if doing a proper refresh() is important | |
| 02:48:16 | dansmith | okay, I'm totally confused | |
| 02:49:07 | melwitt | sorry. I was trying to say I think what we had with the _lookups makes sense considering that the Instance could have been a half-baked object generated from a BuildRequest | |
| 02:49:27 | dansmith | to get the newer instance with .host set, right? | |
| 02:49:40 | melwitt | and only to do something more (like check whether it's orphaned or not to determine whether to _lookup vs refresh()) but I'm not sure that matters | |
| 02:49:44 | melwitt | yeah | |
| 02:50:24 | dansmith | okay but PS79 wouldn't bail if we fail to do that lookup, so I also have to include the bit from PS81 that bails if we fail to lookup because we raced with another delete | |
| 02:51:32 | melwitt | if it's important to do a refresh if real Instance, then we'd have to check for that and do either or depending. and I was going to defer to you on whether we should do that here | |
| 02:52:18 | dansmith | we have to refresh if your comment above the refresh is correct | |
| 02:52:28 | dansmith | otherwise we'll fail to send the delete request to the compute | |
| 02:53:41 | melwitt | I thought the _lookup would serve as the refresh to send it. and then like you said also need the bail out part if it was a race with another delete | |
| 02:54:36 | dansmith | that's what I'm saying | |
| 02:55:05 | dansmith | however, it seemed like I was getting an orphaned instance from that lookup in the previous patchset because I was getting orphaned errors in jenkins, which I no longer am here | |
| 02:57:07 | dansmith | melwitt: you happy with this? http://pastebin.com/QgsXKH1f | |
| 02:57:11 | melwitt | that's unexpected | |
| 02:57:40 | dansmith | melwitt: what is unexpected? the orphaned warnings before? I can link you to the logs :) | |
| 02:58:16 | melwitt | yeah. I believe you, it's just unexpected since I just looked through _lookup_instance and didn't think I saw a way it could get an orphaned instance loaded there | |
| 02:59:09 | dansmith | actually, let me go back one more rev hang on | |
| 02:59:14 | dansmith | but, is the above what you're thinking? | |
| 02:59:17 | melwitt | dansmith: and yes that code is what I was thinking. but I'm concerned about the orphaned errors (although in the IBM CI it shows we still have them, maybe less of a window). I'm going to look through the logs now | |
| 03:00:08 | dansmith | melwitt: okay I got it | |
| 03:00:29 | dansmith | melwitt: PS79 we were getting failure trying to do instance.uuid from the return of _lookup, which can be None | |
| 03:00:48 | dansmith | since I wasn't bailing there, I was using that instance (which had _context) if it was good, and hitting None.uuid if not | |
| 03:00:51 | dansmith | then in the next one, | |
| 03:00:54 | melwitt | dansmith: oh, because you lacked the bailout code | |
| 03:00:57 | dansmith | I used _instance and was getting the orphaned errors | |
| 03:01:04 | dansmith | but maybe got lucky on the race | |
| 03:01:21 | dansmith | so I think the above code, using the _lookup for the refresh and bailing if we get None is probably right | |
| 03:01:24 | melwitt | okay, so the code in the paste should catch all of those cases. I think | |
| 03:01:27 | dansmith | yeah | |
| 03:01:48 | dansmith | the dep chain for this is like five deep so I just can't keep the sequencing of the errors in my head I guess | |
| 03:02:10 | openstackgerrit | Dan Smith proposed openstack/nova: Move instance creation to conductor https://review.openstack.org/319379 | |
| 03:03:31 | dansmith | and looks like the lowest dep is going to get kicked out of the gate.. yay | |
| 03:03:36 | dansmith | #fml | |
| 03:03:53 | melwitt | guh | |
| 03:11:53 | dansmith | I'm still kinda unsure.. this must be a fantastic race that we only hit occasionally | |
| 03:12:18 | dansmith | because I think it ran a bunch of times in a row without hitting it when I was dicking with the listing patch | |
| 03:12:29 | dansmith | but whatever | |
| 03:15:35 | melwitt | back when I was working on the patch to pass the cells v1 job, it failed often enough with the ObjectActionError in that block that I knew to add something. guess I was "lucky" | |
| 03:16:00 | dansmith | right but that was a race, | |
| 03:16:08 | dansmith | this is a race in the race handler :) | |
| 03:16:43 | melwitt | yes. fwiw it got really stable after adding the catch block, it's just the instance.refresh() wasn't really doing anything | |
| 03:16:51 | dansmith | I mean, I'm not saying this doesn't make sense, I'm just wondering why murphy hates me | |
| 03:17:04 | melwitt | yeah, I get you | |
| 03:17:31 | dansmith | I think it was, but only in the cases where we get there from a buildrequest and when we didn't race with another delete right? | |
| 03:19:08 | melwitt | hm, yeah. I guess it would have had an orphaned error itself when it tried to reload itself if it came from a build request | |
| 03:19:23 | dansmith | at least those deps got reset so I get another go at the gate without a recheck | |
| 03:19:36 | dansmith | even though it estimates 43h until completion :/ | |
| 03:20:24 | melwitt | it's showing 1 hour estimate on my view for what's running right now | |
| 03:20:51 | dansmith | it's now 15h for those | |
| 03:20:52 | dansmith | 19: (421228,2) Add --verbose option to discover_hosts command (4h26m/++~~~~~~~/rem:15h43m) | |
| 03:21:26 | dansmith | 1h for this conductor patch | |
| 03:21:35 | melwitt | oh, I see | |
| 03:22:02 | melwitt | the gate was nuts today | |
| 03:22:16 | melwitt | (busy) | |
| 03:22:26 | dansmith | the queues aren't very long right now, but it's still estimating long runtimes, | |
| 03:22:31 | dansmith | so something might be failing a lot | |
| 03:25:58 | dansmith | it's dropping. maybe just a big recalculation and pessimism | |
| 03:26:28 | melwitt | haha | |
| 03:26:55 | melwitt | yeah, I was thinking, most of what's in the gate has only a 1 hr wait left, but I assume that doesn't account for resets | |
| 03:29:15 | melwitt | bbiab | |
| 03:36:48 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova: Updated from global requirements https://review.openstack.org/424828 | |
| 03:38:03 | openstackgerrit | Takashi NATSUME proposed openstack/nova: api-ref: Fix parameters and examples in aggregate API https://review.openstack.org/425512 | |
| 04:05:09 | openstackgerrit | ChangBo Guo(gcb) proposed openstack/nova: Enable more virt.xenapi tests on Python 3 https://review.openstack.org/414421 | |
| 04:16:22 | dansmith | melwitt: powerkvm passed | |
| 04:16:37 | dansmith | but jenkins is going to fail | |
| 04:16:47 | melwitt | ah, I see that. checking what it is | |
| 04:17:21 | melwitt | it's that identity error again from keystone. I've seen that before but I dunno if anyone's documented it in a bug yet | |
| 04:18:01 | dansmith | see the sql failure? | |
| 04:18:08 | dansmith | http://logs.openstack.org/79/319379/82/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/72d1a95/logs/screen-n-api.txt.gz?level=TRACE#_2017-01-26_04_09_09_706 | |
| 04:18:20 | dansmith | so fingers crossed it's not related | |
| 04:18:29 | dansmith | but it means I have to recheck uh-gain | |
| 04:18:54 | melwitt | yeah, looking at those now. weird | |
| 04:20:00 | melwitt | I've seen sql errors on other failed runs before lately | |
| 04:21:13 | dansmith | well, | |
| 04:21:26 | dansmith | I'm not going to be able to stay up for the end of the next run of this | |
| 04:21:45 | dansmith | so I hope that we can get this happy tomorrow | |
| 04:23:14 | melwitt | I'll keep an eye on it | |
| 04:24:18 | dansmith | the powervm fail is a secgroup thing, FYI | |
| 04:25:05 | dansmith | melwitt: okay thanks | |
| 04:25:15 | melwitt | oh, why did it run it two on the same PS | |
| 04:25:26 | melwitt | s/two/twice/ | |
| 04:25:36 | melwitt | oh, powervm vs powerkvm | |
| 04:25:44 | dansmith | yeah | |
| 04:25:48 | melwitt | haha, they look too similar to me | |
| 04:25:49 | dansmith | and powervm is the out of tree driver | |
| 04:26:02 | dansmith | and non-voting | |