| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-24 | |||
| 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. | |
| 14:11:21 | cdent | what kind of 'restart' is being done? | |
| 14:11:26 | sean-k-mooney | mdbooth: well we are not exactly restarting it. the application is crashing somehow and reloading | |
| 14:11:46 | mdbooth | cdent: We're not restarting apache. I did ask about that. | |
| 14:11:53 | sean-k-mooney | mdbooth: if we were to restart the container then we definetly would have had a clean env | |
| 14:12:05 | mdbooth | sean-k-mooney: ack | |
| 14:12:18 | cdent | another option is to try touching the nova-placement-api or placement-api file | |
| 14:12:28 | openstackgerrit | Gaudenz Steinlin proposed openstack/nova master: Extend volume for libvirt network volumes (RBD) https://review.openstack.org/613039 | |
| 14:12:34 | cdent | that _may_ cause the daemon process (if that is what you are using) to clean itself up | |
| 14:12:52 | cdent | that's the "normal" way to do code reload and process reload with daemon process based mod-wsgi | |
| 14:13:17 | sean-k-mooney | cdent: updating the time stamp on an "imutable" continer feel kind of hacky | |
| 14:13:25 | cdent | long term: use uwsgi in the container, and have a FEP in some other container | |
| 14:13:36 | cdent | sean-k-mooney: i agree, in the case, just start the container back up | |
| 14:14:01 | cdent | the rules about how containers operate seems to be being selected sort of randomly | |
| 14:14:12 | cdent | in normal container life: if it doesn't work, you kill it and try again | |
| 14:14:15 | sean-k-mooney | so part of the issue is i think httpd does not exit but the apllicaiton cannoth relaod properly so the container wont restart | |
| 14:14:30 | cdent | and you don't _ever_ run something as heavy as apache2 in a container | |
| 14:14:48 | sean-k-mooney | e.g. if the whole thing exploed when we hit the unrecoverable error then docker would just restat the container and we woudl be fine | |
| 14:15:21 | cdent | you might be able to achieve that by no using daemon mode with mod-wsgi | |
| 14:15:30 | cdent | but uwsgi would be easier :) ;) | |
| 14:15:31 | sean-k-mooney | this is happening on Rocky by the way so in osp13 for us downstream | |
| 14:16:16 | cdent | for future reference sean-k-mooney, have you looked at the way placedock works? https://github.com/cdent/placedock | |
| 14:16:21 | sean-k-mooney | for stein + we coudl look at swappng to uwsgi in kolla i guess as an addtional mitigation | |
| 14:16:40 | sean-k-mooney | cdent: i have ran it once | |
| 14:17:01 | sean-k-mooney | i was tring to figure out could i use it with the osc-placement fuctional test without runnign devstack | |
| 14:17:05 | cdent | the set up there is designed to make it easy to have some other thing in the front (like a load balancer or k8s ingress thing) | |
| 14:17:22 | cdent | if the application fails to start, it quits | |