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

sync/async worker support #4447

Merged
merged 17 commits into from
Feb 24, 2016
Merged

sync/async worker support #4447

merged 17 commits into from
Feb 24, 2016

Conversation

atarkowska
Copy link
Member

This PR doesn't resolve download issue, only makes deployment parameters clear and easy configurable

This PR deprecates existing commend line args and allows fully customize web deployment using parameters, see documentation https://www.openmicroscopy.org/site/support/omero5.2-staging/sysadmins/unix/install-web/install-nginx.html

  • Test Default:
    • pip install -r share/web/requirements-py27-nginx.txt
    • pip install futures
    • bin/omero web start
    • test additional params:
      • omero.web.wsgi_workers (2 x NUM_CORES) + 1
      • omero.web.wsgi_threads (2-4 x NUM_CORES)
      • omero.web.application_server.max_requests 500
      • omero.web.wsgi_timeout 30
      • omero.web.wsgi_args additional arguments
  • Test Async:
    • pip install -r share/web/requirements-py27-nginx.txt
    • pip install gevent
    • bin/omero web start
    • test additional params:
      • omero.web.wsgi_workers (2 x NUM_CORES) + 1
      • omero.web.wsgi_worker_connections 1000
      • omero.web.wsgi_worker_class gevent
      • omero.web.application_server.max_requests 0
      • omero.web.wsgi_timeout 30
      • omero.web.wsgi_args Additional arguments

always check ps aux | grep /home/omero/OMERO.server/var/django.pid if parameters were correctly applied

gunicorn -D -p /home/omero/OMERO.server/var/django.pid --bind 127.0.0.1:4080 --workers 5 --threads 5 --timeout 60 --max-requests 500 omeroweb.wsgi:application

or

gunicorn -D -p /home/omero/OMERO.server/var/django.pid --bind 127.0.0.1:4080 --workers 5 --worker-connections 1000 --worker-class gevent --timeout 60 --max-requests 0 omeroweb.wsgi:application

@atarkowska atarkowska closed this Jan 29, 2016
@atarkowska atarkowska reopened this Jan 29, 2016
@atarkowska atarkowska changed the title Custom chunk size sync/async worker support Feb 4, 2016
@atarkowska atarkowska closed this Feb 8, 2016
@atarkowska atarkowska reopened this Feb 8, 2016
@atarkowska atarkowska closed this Feb 8, 2016
@atarkowska atarkowska reopened this Feb 8, 2016
@atarkowska atarkowska closed this Feb 9, 2016
@atarkowska atarkowska reopened this Feb 9, 2016
@atarkowska atarkowska closed this Feb 9, 2016
@atarkowska atarkowska reopened this Feb 9, 2016
@chris-allan
Copy link
Member

All Gunicorn WSGI command line features seem to be working nicely.

Your expectation is that omero.web.wsgi_worker_class gevent is used to test the asynchronous workers, correct? This is missing from the current testing outline in the description.

Given that we are unable to guarantee OMERO.web thread safety, it was never designed or tested in such an environment, I would strongly suggest we label omero.web.wsgi_threads as experimental and for developer use only. Even if we stated this as a goal for the next release, the testing burden would be enormous especially in light of the potential Ice thread interplay. I would apply the same criteria to the use of gevent or eventlet asynchronous configurations.

/cc @joshmoore, @jburel

@atarkowska atarkowska closed this Feb 16, 2016
@atarkowska atarkowska reopened this Feb 16, 2016
@atarkowska
Copy link
Member Author

@chris-allan thank you for your help with testing. I moved recommended settings to development section. Documentation was updated accordingly. We performed brief testing in https://trello.com/c/TLsw5ffl/73-testing-async-workers. Definitely more test needed.

I am sorry worker_class was missing from the description. I am glade was easy to find that in settings directly.

@atarkowska atarkowska closed this Feb 16, 2016
@atarkowska atarkowska reopened this Feb 16, 2016
@chris-allan
Copy link
Member

Definitely wasn't a problem to find in settings directly after I realized it shouldn't be as difficult as I was making it. Since I have significant prior experience with Gunicorn I was adding omero.web.wsgi_args '-k gevent'. No harm in testing both ways of doing things. :)

With respect to testing threaded or eventlet worker modes, I'm less worried about downloads specifically and more worried about the application as a whole. Especially custom template tags. Any shared state in package variables, classes or similar exposes potential race conditions, crashes, and other safety issues when run in a threaded environment.

Quite simply, it is incredibly dangerous to take the amount of code we have in OMERO.web and just assume it is completely thread safe. Especially when that was never a stated design or development consideration to begin with.

Adding separate comments to ome/omero-documentation#1396 regarding the location of experimental feature documentation on the page.

/cc @will-moore

@jburel
Copy link
Member

jburel commented Feb 24, 2016

@hflynn: the change gunicorn>=19.3 will have to reflected in the scripts in omero-install (then the doc) I will take care of it. since we are not yet using the requirements file (due to structure currently in place)

@jburel
Copy link
Member

jburel commented Feb 24, 2016

Merging thanks all

jburel added a commit that referenced this pull request Feb 24, 2016
@jburel jburel merged commit 37b88c6 into ome:develop Feb 24, 2016
@atarkowska atarkowska deleted the fix_download branch February 24, 2016 10:29
@jburel
Copy link
Member

jburel commented Feb 24, 2016

PRs now opened:

@jburel jburel added this to the 5.2.2 milestone Feb 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants