Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crochet doesn’t work with uWSGI #78

Closed
hynek opened this issue Feb 18, 2015 · 20 comments · Fixed by #83
Closed

crochet doesn’t work with uWSGI #78

hynek opened this issue Feb 18, 2015 · 20 comments · Fixed by #83

Comments

@hynek
Copy link

hynek commented Feb 18, 2015

I tried to use crochet with uWSGI and a Pyramid application and a lot of fail has been had.

A few things make it crash with http://glui.me/?d=is6368qf71cqb25/2015-02-18%20at%2013.02.png/ right away like lazy-apps, others make it hang (like starting multiple processes).

That makes me kind of sad, because I really like uWSGI. :-/ Works like a charm with gunicorn so far.

@itamarst
Copy link
Owner

  1. I've seen issues with multiprocessing, which forks Python without doing exec() (see crochet.setup() interacts badly with fork()-without-exec() #71) ; does uWSGI do that?
  2. More generally: if you're using something that starts multiple Python processes, you need to do crochet.setup() in every process. Probably best way to do that is to defer calling crochet.setup() to the WSGI application function itself?

@hynek
Copy link
Author

hynek commented Feb 21, 2015

  1. I think uWSGI is mostly C, but it’s totally possible that it‘s the same pattern.
  2. I’m calling crochet.setup() in the main function that is used by Pyramid to setup the wsgi application. I wouldn’t really know where else to hook in.

@itamarst
Copy link
Owner

I found this:

uWSGI tries to (ab)use the Copy On Write semantics of the fork() call whenever possible. By default it will fork after having loaded your applications to share as much of their memory as possible. If this behavior is undesirable for some reason, use the lazy-apps option. This will instruct uWSGI to load the applications after each worker’s fork(). Beware as there is an older options named lazy that is way more invasive and highly discouraged (it is still here only for backward compatibility)

So see if lazy-apps option fixes it? If so let me know, and leave this issue open so I can document it.

@itamarst
Copy link
Owner

Or, I suppose, if the first time you call crochet.setup() it stores the PID, and then future iterations it compares actual PID to stored PID and reinitializes if necessary...

@hynek
Copy link
Author

hynek commented Feb 21, 2015

I had lazy-apps previously but IIRC it crashed right away. Making crochet.setup() idempotent sound like a good idea though.

@itamarst
Copy link
Owner

Before going down that route, can you provide a minimal reproducer?

@hynek
Copy link
Author

hynek commented Feb 21, 2015

Oh and quitting is a problem too:

Sat Feb 21 20:23:11 2015 - [emperor] stop the uwsgi instance development.ini
Sat Feb 21 20:23:11 2015 - received message 0 from emperor
^C[emperor] *** RAGNAROK ALREADY EVOKED (mercyless in 9 seconds)***
^C[emperor] *** RAGNAROK ALREADY EVOKED (mercyless in 7 seconds)***
^C[emperor] *** RAGNAROK ALREADY EVOKED (mercyless in 7 seconds)***
^C[emperor] *** RAGNAROK ALREADY EVOKED (mercyless in 7 seconds)***
Sat Feb 21 20:23:41 2015 - [emperor] NO MERCY for vassal development.ini !!!
Sat Feb 21 20:23:41 2015 - The Emperor is buried.
Sat Feb 21 20:23:41 2015 - uWSGI worker 3 screams: UAAAAAAH my master disconnected: i will kill myself !!!
Sat Feb 21 20:23:41 2015 - uWSGI worker 1 screams: UAAAAAAH my master disconnected: i will kill myself !!!
Sat Feb 21 20:23:41 2015 - uWSGI worker 4 screams: UAAAAAAH my master disconnected: i will kill myself !!!
Sat Feb 21 20:23:41 2015 - uWSGI worker 2 screams: UAAAAAAH my master disconnected: i will kill myself !!!
Sat Feb 21 20:23:41 2015 - uWSGI worker 5 screams: UAAAAAAH my master disconnected: i will kill myself !!!

@itamarst
Copy link
Owner

FYI crochet.setup() is already idempotent. It just isn't idempotent if you fork after it's been called, and then call it again in forked child, because who would be crazy enough to do that?

@hynek
Copy link
Author

hynek commented Feb 22, 2015

I made an example project:

https://github.com/hynek/wsgi_test

I use --lazy-apps and it explodes as I remembered it:

Traceback (most recent call last):
  File "wsgi.py", line 7, in <module>
    from crochet import no_setup, setup, wait_for
  File "/Users/hynek/.virtualenvs/wsgi_test/lib/python2.7/site-packages/crochet/__init__.py", line 25, in <module>
    from ._shutdown import _watchdog, register
  File "/Users/hynek/.virtualenvs/wsgi_test/lib/python2.7/site-packages/crochet/_shutdown.py", line 59, in <module>
    if t.name == "MainThread"][0], _registry.run)
IndexError: list index out of range

Which is a bummer, because I need that option for the Sybase driver.

The emperor mode is a whole different beast entirely because it uses PasteDeploy which I’m too dumb to quickly get an example together, HTH anyway.

Unfortunately I wasn’t able to reproduce the hangs either, but baby steps.

@itamarst
Copy link
Owner

That parts looks like what #69; the logic for detecting main thread is quite iffy.

@itamarst
Copy link
Owner

Thanks for the example, I'll take a look.

@itamarst
Copy link
Owner

I opened an issue (#79) for the MainThread thing. I'll leave this issue to cover anything else uncovered by testing your example.

@itamarst
Copy link
Owner

itamarst commented Mar 9, 2015

exarkun suggested pthread_atfork as a way of dealing with fork-without-exec.

@dnephin
Copy link

dnephin commented Mar 30, 2015

I'm looking into a solution for this issue.

Running in uwsgi the thread names (at the time of this error) are: ['uWSGIWorker1Core0', 'uWSGIAutoReloader'], where Worker1 is going to change for every worker.

What do you think about allowing for the name of the main thread (or some prefix) to be set from the environment? So the watchdog would be able to use the worker thread to register shutdown?

I also wonder if this registration could happen as part of setup() instead? That might allow for customization of the registration without using environment variables.

@itamarst
Copy link
Owner

That's one possibility. Another is covered by #79, which has my current thoughts on how this could be solved - getting rid of the wait-for-thread shutdown mechanism altogether and just rely on Python to shut things down. A workaround until I work this out is to just rename the appropriate thread "MainThread" (just a simple setattr would do) before importing crochet.

@dnephin
Copy link

dnephin commented Mar 31, 2015

I'm not sure what other side-effect there might be if I go renaming the uwsgi threads, so I'd rather avoid that. I'll submit a PR with a small refactor to allow a thread selector to be passed to setup() and you can let me know what you think about it.

@itamarst
Copy link
Owner

itamarst commented Apr 4, 2015

Rather than a thread selector, how about allowing passing a function to setup() that returns True if it's OK to shutdown? The implementation is polling based anyway, so this way you can have any mechanism you like to determine shutdown, including a manual shutdown-now function.

@dnephin
Copy link

dnephin commented Apr 4, 2015

That sounds good, and should be easier to implement as well.

@itamarst
Copy link
Owner

itamarst commented Apr 4, 2015

Cool, I'd love a PR if you've got the time.

@itamarst itamarst changed the title crotchet doesn’t work with uWSGI crochet doesn’t work with uWSGI Apr 25, 2015
@itamarst
Copy link
Owner

itamarst commented May 6, 2015

I have verified that both normal and lazy-app mode in example provided by @hynek work with latest Crochet master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants