| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-24 | |||
| 12:28:57 | openstackgerrit | Daniel Abad proposed openstack/nova master: Fix ironic client ironic_url deprecation warning https://review.openstack.org/611872 | |
| 12:39:27 | openstackgerrit | Gaudenz Steinlin proposed openstack/nova master: Ignore misleading resource updates from virt driver https://review.openstack.org/523006 | |
| 12:44:16 | jangutter | sean-k-mooney: one more clarification, when you say "filtered host-info", what's the filter that's applied? Is it just filtering based on the plugin info gathered on the compute node, filtering out out-of-range plugins? | |
| 12:45:57 | sean-k-mooney | basicaly the idea was that nova would prefilter the host info object on 2 factor | |
| 12:46:19 | sean-k-mooney | first what plugins/viftypes were supported by the hypervior on that node | |
| 12:46:37 | sean-k-mooney | and second any aspect of the guest that would resitct what vif types could be used. | |
| 12:47:16 | jangutter | sean-k-mooney: I see, only info locally available at the compute node at the time, analogous to "capabilities". | |
| 12:47:30 | sean-k-mooney | e.g. vhost-user requires hugepages to work so if the flavor did not have a hugepage request it would be removed form the list | |
| 12:47:41 | sean-k-mooney | yep | |
| 12:49:51 | sean-k-mooney | the main motivaitoin is to enable nova to say based on my knoladge of the hypervior and the instance request(flavor and image metatdata) this is the set of vif types i could support | |
| 12:50:38 | sean-k-mooney | and then neutron can say ok form that set i can support Y and select an optimal vif type to use | |
| 12:50:57 | stephenfin | np | |
| 12:51:17 | sean-k-mooney | once we have that capablity we can potainilly schedule on that in the future too | |
| 12:52:02 | jangutter | sean-k-mooney: right, makes sense and saves a round-trip with mis-scheduling or a failed portbinding. | |
| 12:52:33 | sean-k-mooney | yep or worse in the vhost user case where the vm boots with no error and no networking | |
| 12:56:05 | jangutter | sean-k-mooney: even the qemu error in libvirt is tricky to trace and very misleading in that case. | |
| 12:56:50 | sean-k-mooney | jangutter: qemu does not provide an error in that case at least it did not in the past | |
| 12:57:30 | sean-k-mooney | the only error i have ever seen for this is a debug only error in dpdk logs related to not being able to map the memory | |
| 12:58:31 | sean-k-mooney | but in anycase it would allow us on both the nova and neutron side to filter down to only inteface we think should work instead of relying on the operator/deployer to get this right | |
| 12:59:08 | jangutter | sean-k-mooney: I remember finding something in the qemu logs about "falling back on userspace virtio" when that happened. No hard error. | |
| 13:00:38 | sean-k-mooney | really well the fallback does not work so i guess its nice they tried but ya i think we have all been bit by that at some point if we have used vhost-user | |
| 13:02:13 | jangutter | sean-k-mooney: one day, there'll be unscarred users, developers and operators. | |
| 13:04:56 | sean-k-mooney | jangutter: you mean when ai ban humans form coding and do it all themselves i totally agree | |
| 13:05:14 | sean-k-mooney | that or when they kill all the users ... | |
| 13:18:51 | openstackgerrit | sean mooney proposed openstack/nova master: harden placement init under wsgi https://review.openstack.org/610034 | |
| 13:36:29 | sean-k-mooney | zzzeek: mdbooth cdent so regarding https://review.openstack.org/#/c/610034/4/nova/api/openstack/placement/db_api.py i think we have 3 paths forward | |
| 13:36:32 | mdbooth | sean-k-mooney zzzeek: Continuing our previously downstream discussion of https://review.openstack.org/#/c/610034/ | |
| 13:37:22 | sean-k-mooney | one agree my code is awsome and merge it. 2 add a flag on the consumres side to track if we have configured it allready or 3 extend oslo.db to allow reconfiguring a transation_context that has been started | |
| 13:37:43 | mdbooth | sean-k-mooney: zzzeek to confirm, but I suspect 3 is not a thing | |
| 13:37:55 | mdbooth | I was going to propose a 2 phase approach: | |
| 13:38:44 | mdbooth | 1. Set a flag in the module on configuration, assert configuration only happens once, emit an unconditional warning on reconfiguration that reconfiguration did not happen. | |
| 13:39:36 | cdent | can someone explain what's wrong with option sean's option 'one'? | |
| 13:39:41 | mdbooth | 2. Update all decorators which currently close over placement_context_manager to call get_placement_context_manager(), and go with the original plan of creating a new one on reconfiguration. | |
| 13:40:19 | mdbooth | cdent: The TypeError will bite us when there's a bug in oslo.db, or it's changed inadvertently as code is moved around. It's not a deliberate API. | |
| 13:40:34 | mdbooth | And the configure flag is trivial to implement and better. | |
| 13:40:53 | cdent | do we have to do step 2? that's idiomatic throughout all of placement and nova | |
| 13:41:12 | cdent | it is perhaps messy, but a considerable change | |
| 13:41:17 | mdbooth | cdent: We don't have to do step 2, but it's the only way I can think of that we'll get reconfiguration across a restart. | |
| 13:41:33 | mdbooth | Agree, hence the existence of step 1. | |
| 13:42:08 | sean-k-mooney | cdent: well we could still keep decorator we just need to have a different one that does the dispatch to the current instance of the global rather then the one that was bound on import | |
| 13:42:14 | cdent | is "reconfiguration across a restart[1]" required? | |
| 13:42:22 | cdent | [1] I think maybe you mean reload in apache terms? | |
| 13:43:16 | mdbooth | cdent: Yeah. I understood it was required, but I'm prepared to hear it's not. | |
| 13:43:19 | sean-k-mooney | cdent: ya i was thinking that an operator may have changed the config a some point and when there awas a failure it could pickup those chages | |
| 13:43:35 | sean-k-mooney | cdent: im not sure its requried but it was an question i had | |
| 13:43:36 | mdbooth | If it's not required, there's no reason for such a noisy change. | |
| 13:43:59 | mdbooth | I think the warning makes sense, though. | |
| 13:44:02 | sean-k-mooney | e.g. is skipping reconfiguration vaild always | |
| 13:44:26 | sean-k-mooney | mdbooth: ya i had a debug level log on the placement version of the chagne | |
| 13:44:41 | sean-k-mooney | nova did not have the logger so i left it out of the nova version | |
| 13:45:33 | cdent | has anyone checked to see if mod-wsgi's behavior can be changed to be more like uwsgi's? | |
| 13:46:35 | sean-k-mooney | no but i had considerd seeing if we could change the kolla images to use uwsgi also but no time | |
| 13:47:14 | sean-k-mooney | i understand they did this for performance reasons in mod-wsgi to have quicker reloads but it seam wrong to me | |
| 13:47:29 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix min config value for shutdown_timeout option https://review.openstack.org/613028 | |
| 13:48:30 | sean-k-mooney | this seams to be the only related thin in there configs https://modwsgi.readthedocs.io/en/develop/configuration-directives/WSGIScriptReloading.html | |
| 13:52:03 | mdbooth | cdent: This could easily be mechanical, btw | |
| 13:52:24 | mdbooth | cdent: I'm going to chuck up a quick POC/strawman | |
| 13:53:05 | cdent | mdbooth: I'm not direclty opposed to changing it, I'm just much too familiar with being able to change things | |
| 13:53:11 | cdent | being difficult | |
| 13:53:23 | mdbooth | cdent: Hehe, we're on the same page :) | |
| 13:53:42 | cdent | here we go: https://modwsgi.readthedocs.io/en/develop/user-guides/application-issues.html#reloading-python-interpreters | |
| 13:53:45 | mdbooth | cdent: This is why, downstream, I prioritised the deployment framework workaround for the same issue :) | |
| 13:55:30 | cdent | looks like that reloading solution has some issues with c extensions | |
| 13:56:45 | cdent | sean-k-mooney: did you see the link ^ | |
| 13:57:24 | sean-k-mooney | yes reading. the interperter reload option was removed in veriosn 2.0 so we cant use it anyway | |
| 13:58:05 | cdent | "As an alternative, daemon mode of mod_wsgi should be used and the “Process” reload mechanism added with mod_wsgi 2.0."? | |
| 13:58:29 | mdbooth | cdent sean-k-mooney: I just threw something together. Running a smoke test before sharing. | |
| 13:59:24 | cdent | I continue to think that we're trying to fix a problem in placement when it should be fix in how mod-wsgi is being managed | |
| 13:59:37 | sean-k-mooney | mdbooth: ok but the decoror change shoudl be really trivial i know it will work but the orginical question is it needed | |
| 14:00:23 | sean-k-mooney | cdent: well its more a question of what is the abstract machine we expect to execut the code in | |
| 14:00:56 | cdent | sean-k-mooney: sure, and WSGI, as a protocol, has some pretty simple guidelines | |
| 14:01:14 | cdent | it is, by design, super fast to start up a new proecess wtih the application in it | |
| 14:01:30 | mdbooth | The little I read didn't seem to define whether you get a new python vm or not. | |
| 14:01:45 | cdent | because it just assumes you do | |
| 14:01:54 | mdbooth | assume != define | |
| 14:01:56 | cdent | that's kind of part of the WSGI-nature | |
| 14:02:13 | cdent | sure, but you're asking placement to take on additional complexity in the wrong place | |
| 14:02:30 | mdbooth | Unless we're saying that mod_wsgi is architecturally broken and shouldn't be used by anyone? | |
| 14:02:42 | cdent | WSGI exists so that http server-related concerns can exist outside the wsgi application code | |
| 14:02:47 | mdbooth | That could be true, I wouldn't know | |
| 14:02:53 | cdent | I do say that, these days :) | |
| 14:03:01 | cdent | and so do many other people | |
| 14:03:23 | mdbooth | cdent: Are those the same folks using undefined behaviour? :P | |
| 14:03:54 | cdent | they are people who don't want the wsgi server having any impact on the wsgi application, which mod-wsgi always has | |
| 14:04:09 | cdent | but for a long time it was the only performant choice | |
| 14:04:15 | sean-k-mooney | mdbooth: how mod wsgi is work i would say is falling int the same camp as undefiend behavior in c/c++ | |
| 14:05:19 | cdent | I'm happy to consider changes in placement for all of this stuff, but it would make me much happier if there was also visible effort to inquire with Graham about whether there are ways to achieve the same thing in mod-wsgi | |
| 14:05:34 | cdent | we seem to be trying to take the local path, when a more global path _might_ be the right thing | |
| 14:07:01 | sean-k-mooney | cdent: well long term i think relaoding a script by spawning an entirely seperate prociess with its own pyton interperet would be the correct thing to do in mod_wsgi | |
| 14:07:40 | sean-k-mooney | that said trippleo also shoudl not be starting the placemetn api continer while upgradeing tht database | |
| 14:08:12 | mdbooth | sean-k-mooney: Yep, of course. But that's not the only reason why might fail on startup. This is still an issue. | |
| 14:08:13 | sean-k-mooney | short term we proably need a local solution | |
| 14:08:28 | mdbooth | Super short term is the tripleo fix. | |
| 14:09:00 | sean-k-mooney | mdbooth: well the tripleo fix would have been correct in any case | |
| 14:09:05 | cdent | I don't feel like I have all the info | |
| 14:09:08 | mdbooth | sean-k-mooney: ack | |
| 14:09:37 | mdbooth | cdent: We have a downstream failure because we're starting placement while still running db_sync on its db | |
| 14:09:39 | cdent | but I also don't feel like we (the three of us) have all the info about how mod-wsgi works | |
| 14:10:28 | mdbooth | When we restart it, we get a failure because placement_context_manager is already configured. | |
| 14:11:07 | mdbooth | So obviously we shouldn't be doing that, but it highlights that *any* restart of placement like this will fail for the same reason. | |