Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-10-26
15:26:34 cdent either the attributeerror of the keystone error needs to be handled in some approrpriate fashion
15:26:48 cdent s/of/or/
15:27:17 fried_rice It's really just so we can know what actually happened rather than having to rely on the tribal knowledge that when a placement helper method returns None, it's probably because comm failed.
15:27:49 cdent (re that link: my eyes!!!!)
15:27:49 fried_rice Getting rid of @safe_connect is mostly about ^ that, not about doing something more appropriate with the now-not-as-mysterious exception. That would be a separate thing.
15:28:54 fried_rice cdent: hold on, we've got some of my log coloring experimental code on that server, I think there's a secret option to change themes...
15:29:42 cdent sure, but the reason safe_connect existed in the first place was to do some kind of "right thing" when comm's failed. what's wrong here is that safe_connect is broken, not that it exists
15:30:02 cdent safe_connect was supposed provided away for interactions with the placement api to be correctly retried (or logged) when things went awry
15:30:18 cdent somewhere along the way it lost that and became simply a bad thing that returned None when it shouldn't
15:30:51 cdent There's no doubt (at all) that it is a bad thing, now. But the lost of the intention is also a bad thing.
15:31:26 cdent If we're cool with the idea of removing it (because its masks real things that need to be fixed) will to stuff actually being fixed, cool
15:31:26 fried_rice in this case, the next spin through the periodic will hopefully work.
15:31:48 cdent that seems an odd approach to take, to me
15:31:57 fried_rice That's "retry"
15:32:04 fried_rice or if this call happened as the result of an actual operation, we've got retries built in, IIRC. Sec...
15:32:06 cdent because the next periodic could be many minutes away if the deployer has chosen to do so
15:32:22 fried_rice Yeah, and if an operation isn't happening, who cares
15:32:35 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Pass request_spec from compute to cell conductor on reschedule https://review.openstack.org/582417
15:33:06 cdent if the failed request was e.g., updating inventory, scheduling will go wrong during that window
15:33:55 cdent What I'm trying to get at here is that I suspect (actually, am confident that) the system is more complex than "just let the expceptions rise" can bear
15:33:57 fried_rice So this is all happening under the umbrella of _update, which has a retry around it for conflicts
15:34:19 cdent but what about for a network partition?
15:35:33 cdent (or things that look like a network partition (such as the load balance being reconfigured))
15:36:16 fried_rice Look, I'm not disputing that we could do a better job of handling placement comm errors in this whole flow (i.e. the resource tracker). But first we have to know we're having a placement comm error.
15:36:36 fried_rice If we want to put in retries at some level based on ClientException, I'm all behind that.
15:36:42 fried_rice But that's not what this patch is for.
15:38:41 cdent Okay, but "handling placement comm errors" is why safe_connect was created, so simply killing it is probably not the right approach. Fixing it probably is.
15:39:12 cdent I'm harping on it because it is clear that the reasson for its existence has now been lost to the sands of time (because somewhere along the way it borked)
15:39:41 cdent But we seem to want to say "it sucks, let's just kill it" (at least jokingly) and that's not really sufficient in a distributed system.
15:41:00 cdent These issues will come back around again, after safe_connect has gone, and we'll realize "you know, we need to create this thing that allows us to safely connect"
15:41:08 fried_rice We have made several assays toward fixing it. It can't be done. Because it's at too low a level in the call stack.
15:41:12 SteelyDan cdent: actually safe_connect was really to handle the "have they set up placement or not" situation
15:41:33 SteelyDan which is why we've been talking about removing it entirely since placement became required
15:41:50 cdent SteelyDan: that was _one_ of the reasons, and yes, the very first reason, but very quickly it took on "deal with errors from keystoneclient"
15:42:31 SteelyDan yes, it has evolved
15:42:42 openstackgerrit Eric Fried proposed openstack/nova master: Kill @safe_connect in _get_provider_traits https://review.openstack.org/613613
15:42:46 cdent we had: is it in the service catalog, then can I reach it
15:42:52 fried_rice cdent: added words clarifying intent --^
15:43:32 cdent thanks
15:44:31 openstackgerrit sean mooney proposed openstack/nova master: harden placement init under wsgi https://review.openstack.org/610034
15:44:34 cdent I'm trying really hard to not to think to much about the resource tracker lately but sometimes I get sucked in.
15:45:34 sean-k-mooney cdent: ^ uses the run_once decorator
15:45:38 fried_rice cdent: Anything we would do to @safe_connect, beyond making it essentially a no-op shell (which isn't better than removing it IMO), would also entail a thorough audit of everywhere it is used, which is a nearly impossible task due to how many different ways and in how many layers things get called.
15:45:38 fried_rice For example, if we decided to make it retry, we would have to restrict the decorator to *only* single primitive API calls. We would have to remove it from all the methods that do more complex things e.g. involving multiple API calls.
15:46:02 cdent sean-k-mooney: yeah, thanks, reading
15:46:25 sean-k-mooney cdent: ill propose the placement version shortly if people are fine with it
15:47:07 cdent fried_rice: yes, I know, thus why I'm trying really hard not to think about the resource tracker lately. It was already really complicated before placement, and we somehow managed to make it worse, not better.
15:47:28 fried_rice I have made a couple of attempts, and I think SteelyDan has as well, to do something to @safe_connect itself to improve it. At some point in a review leakypipes said we should just target one usage at a time and get rid of it. Which so far has been the only approach that has gotten us to actually merge code around this.
15:47:45 SteelyDan yup
15:47:54 fried_rice cdent: I still hope that it will be better in the long run, once we've purged all the legacy stuff from this painful transitional period.
15:48:09 fried_rice like @safe_connect :P
15:48:34 fried_rice and stuff gibi has been doing lately has cleaned up some of that legacy gorp too.
15:48:37 fried_rice So we're getting there.
15:48:49 cdent well, just to cover my ass here: If I had my druthers we never would have had safe_connect
15:49:09 cdent but we gots we gots
15:49:12 cdent i'll cope
15:49:28 fried_rice My dad used to say, "If frogs had wings, they wouldn't bump their asses so much."
15:49:45 fried_rice (I made that sound like he's dead or something. He would still say it, given the slightest opportunity.)
15:49:50 cdent heh
15:55:50 cdent sean-k-mooney: have you run the functional tests on that change? I think you're going to run into trouble with the databse fixture
15:55:58 spatel sean-k-mooney: is there a way to find real instance hostname using virsh list command?
15:57:05 spatel currently i am doing "virsh dumpxml instance-00000387 | grep nova:name"
15:57:14 spatel i wish it has build it function
15:58:06 sean-k-mooney cdent: am no but i will and see what it does not like
15:58:15 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix min config value for shutdown_timeout option https://review.openstack.org/613028
15:58:22 cdent sean-k-mooney: i'm doing it now, will leave a comment with the reasults
15:59:33 sean-k-mooney spatel: no grep is your friend in this case
15:59:50 spatel :) thanks
16:01:23 fried_rice cdent: btw, that log server has the ANSI color coding patch (which isn't working 100% properly - some codes don't like to turn off), not the choose-your-theme patch, so you're SOL, sorry.
16:01:51 cdent fair enough, as before, I'll cope
16:02:50 fried_rice if we ever reach a point where you're having to look at PowerVM CI logs more than once a year, I'll take that patch offa there :)
16:03:08 cdent ✔
16:06:56 openstackgerrit Martin Midolesov proposed openstack/nova master: vmware:PropertyCollector for caching instance properties https://review.openstack.org/608278
16:06:56 openstackgerrit Martin Midolesov proposed openstack/nova master: VMware: Expose esx hosts to Openstack https://review.openstack.org/613626
16:14:36 openstackgerrit Martin Midolesov proposed openstack/nova master: VMware: Expose esx hosts to Openstack https://review.openstack.org/613626
16:15:11 openstackgerrit Matt Riedemann proposed openstack/nova master: Migrate "reboot an instance" user guide docs https://review.openstack.org/612730
16:17:28 mriedem fried_rice: gibi: do you see something here that i don't? https://review.openstack.org/#/c/605785/16/nova/tests/functional/test_servers.py@5275
16:17:49 mriedem if i'm wrong, i'll +W
16:18:23 openstackgerrit Merged openstack/python-novaclient master: Fixes Python3 issue in decoding password https://review.openstack.org/604870
16:19:55 fried_rice mriedem: I didn't scrutinize tbh. If it is a redundant test, I figgered it could be removed in a fup.
16:21:54 mriedem btw, on that safe_connect thing, couldn't all of the same arguments be made for all other services nova talks to if we're worried about network drops? so that would be at least: keystone, glance, cinder, neutron, *barbican
16:22:06 mriedem i think the ironic driver already has it's own retry wrapper
16:22:24 cdent yes?
16:23:09 cdent we assume a lot that's can to be less easy to assume in this like edge and other blech like that
16:23:15 cdent s/can/going/
16:23:32 mriedem oh i'm sure
16:23:54 mriedem ksa already has retry stuff in it too right?
16:23:57 mordred yup
16:24:01 melwitt
16:24:16 mordred and in sdk we have a structure for expressing per-service error codes that are safe to do automatic retries on
16:24:39 cdent this whole day has been a slow setup to allow mordred to do an sdk advertisement
16:24:46 mordred cdent: isn't htat what most days are?
16:24:51 mriedem we just need to rip and replace all nova's internal client things with the sdk, done!
16:25:09 mordred coming soon to a gerrit near you!
16:26:03 mordred (honestly, I've got one more patch i need to write sdk-side, then I want to make y'all a couple of sake-of-argument patches so we can look at what it might be like and whether it's worth working on in earnest)
16:27:12 mriedem working on what? sdk in nova?
16:27:31 mriedem back in the first YVR summit i think dean said the service projects shouldn't use the sdk, but that was a long time ago
16:27:50 mriedem i think for perf reasons
16:27:56 mriedem but it seems kinda dumb not to
16:29:06 leakypipes gibi, mriedem: anyone got the link to gibi's patch series that removes the doubled-up allocation cruft for legacy evacuate code paths?

Earlier   Later