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

Wrong app initialization in wsgi.py #240

Closed
cehbrecht opened this issue May 3, 2018 · 5 comments
Closed

Wrong app initialization in wsgi.py #240

cehbrecht opened this issue May 3, 2018 · 5 comments

Comments

@cehbrecht
Copy link
Member

This bug is reported by CRIM:

I recently started working on the PAVICS project, and I have a bug to submit you, but I’m not sure where is the best place/project to submit it.

We have a common bug with the WPS services built on pywps, such as Malleefowl, Flyingpigeon and Hummingbird.

On each WPS request we perform on those services, an additional file descriptor gets opened on the services’ log file, for example, inside the Flyingpigeon Docker container, it’s the file /opt/birdhouse/var/log/pywps/flyingpigeon.log.

Consequently, all log output gets duplicated by the number of times the gunicorn worker had previously processed requests… filling up our hard drives very rapidly.

My quick workaround is to add “--max-requests 1” to the gunicorn command line, so the workers are restarted after each requests.

But after longer investigation, I found out that the root cause of the problem is in wsgi.py of those services.

For example, Flyingpigeon is currently written like this:

import os
from pywps.app.Service import Service
 
from .processes import processes
 
def application(environ, start_response):
    app = create_app()
    return app(environ, start_response)
 
 
def create_app(cfgfiles=None):
    config_files = [os.path.join(os.path.dirname(__file__), 'default.cfg')]
    if cfgfiles:
        config_files.extend(cfgfiles)
    if 'PYWPS_CFG' in os.environ:
        config_files.append(os.environ['PYWPS_CFG'])
    service = Service(processes=processes, cfgfiles=config_files)
    return service

Unfortunately, create_app() is called on every request handled by the worker. Rewriting the wsgi.py like this fixes the problem:

import os
from pywps.app.Service import Service
 
from .processes import processes
 
def create_app(cfgfiles=None):
    config_files = [os.path.join(os.path.dirname(__file__), 'default.cfg')]
    if cfgfiles:
        config_files.extend(cfgfiles)
    if 'PYWPS_CFG' in os.environ:
        config_files.append(os.environ['PYWPS_CFG'])
    service = Service(processes=processes, cfgfiles=config_files)
    return service
 
application = create_app()

Thanks!

@cehbrecht cehbrecht added the bug label May 3, 2018
@cehbrecht cehbrecht added this to the 1.3 milestone May 3, 2018
@cehbrecht cehbrecht self-assigned this May 3, 2018
@cehbrecht cehbrecht added this to To Do in Dar-es-Salaam: September 2018 Release via automation May 3, 2018
@cehbrecht
Copy link
Member Author

This bug needs to be fixed in all WPS birds. We can keep the main bug report in flyingpigeon.

@cehbrecht
Copy link
Member Author

Would be nice to have a common wsgi.py app in pywps which can dynamically load the configured processes. See geopython/pywps#118

@cehbrecht
Copy link
Member Author

@ldperron I have fixed this now in the birds used by PAVICS. Other birds will follow. Still there is a duplication of log entries ... maybe due to the number of gunicorn workers. You can change in addition the log-level maybe just to WARN to reduce the log files. The pywps log files have no auto-rotation ... needs to be added on the unix system.

You can merge or cherry-pick the changes.

cehbrecht added a commit that referenced this issue May 3, 2018
@ldperron
Copy link
Contributor

ldperron commented May 3, 2018

Thanks! We will stay with the workaround for now, but I will make sure to remove the workaround when we upgrade any of the birds.

@ldperron
Copy link
Contributor

ldperron commented May 4, 2018

Finally, the workaround using --max-requests 1 was a very bad idea. It totally breaks async execution of WPS processes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

2 participants