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

[16.07] Create a uWSGI postfork function registry and start the tool conf watcher thread post-fork #2774

Merged
merged 8 commits into from Aug 15, 2016

Conversation

Projects
None yet
3 participants
@natefoo
Copy link
Member

commented Aug 11, 2016

process_is_uwsgi being in galaxy.util.postfork is maybe not the best place, but it's not any worse than in galaxy.config where it was.

This will hopefully fix the dead uWSGI worker issue we've been having on Main.

xref unbit/uwsgi#1149

@dannon

This comment has been minimized.

Copy link
Member

commented Aug 11, 2016

@natefoo Nice! Checking it out now.

@dannon

This comment has been minimized.

Copy link
Member

commented Aug 12, 2016

@natefoo With the addition of natefoo#4, this solves it for me. I added a bunch of extra debugging and verified that the update manager was the only other thread running (again, for my configuration) at launch time.

Edit: Added trimmed down debugging to launch time, just prior to webapp return/fork to make it easier to find potentially problematic threads down the road.

Add debugging to indicate running threads just prior to prefork, to m…
…ake finding and fixing of these easier in the future.
@natefoo

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2016

Ah, that should be disabled by default. Did you explicitly enable it? If not, sounds like there's a bug.

Thanks! Glad to know we can confirm that it's the threads that's causing these issues.

@dannon

This comment has been minimized.

Copy link
Member

commented Aug 15, 2016

@natefoo That'd be enable_tool_shed_check right? (answering my own question: yes, and I did have it enabled on that test box)

@natefoo

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2016

Ok good. It (like all threads) needed to be started postfork as well, so your change is good regardless. Should I wait for your uwsgidecorators investigation to merge natefoo#4 or can I merge as is? It's good from my perspective.

Merge pull request #4 from dannon/uwsgi-fork
Force update repository manager to launch postfork.
@natefoo

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2016

Saw your comment on the PR so I merged it. Thanks for working on this!

@dannon

This comment has been minimized.

Copy link
Member

commented Aug 15, 2016

@natefoo Want to include the warning in natefoo#5?

dannon and others added some commits Aug 15, 2016

Merge pull request #5 from dannon/uwsgi-fork
Add a warning when it is detected that a process is uwsgi based but u…

@dannon dannon merged commit d87fbdd into galaxyproject:release_16.07 Aug 15, 2016

0 of 4 checks passed

api test Test started.
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
framework test Test started.
Details
toolshed test Test started.
Details
@nsoranzo

This comment has been minimized.

Copy link
Member

commented Aug 15, 2016

I still have workers failing to start if watch_tools = polling , see https://gist.github.com/nsoranzo/41b5df22b73396421a168fa0c23f5a17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.