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

Introduce gevent request timeout #310

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .prod.env
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,5 @@ GLOBAL_STORAGE=10737418240

# Gunicorn server socket
PORT=5000

GEVENT_WORKER=True
11 changes: 7 additions & 4 deletions server/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@
# Modules that had direct imports (NOT patched): ['urllib3.util, 'urllib3.util.ssl']"
# which comes from using requests (its deps) lib in webhooks

import os
from random import randint
from mergin.config import Configuration as MainConfig

if not os.getenv("NO_MONKEY_PATCH", False):
Copy link
Contributor

@MarcelGeo MarcelGeo Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to refactor this NO_MONKEY_PATCH variable in whole system? I'm not sure, if we want to use it in private repo.

if MainConfig.GEVENT_WORKER:
import gevent.monkey
import psycogreen.gevent

gevent.monkey.patch_all()
psycogreen.gevent.patch_psycopg()

gevent.monkey.patch_all(subprocess=True)

from random import randint
from celery.schedules import crontab
from mergin.app import create_app
from mergin.auth.tasks import anonymize_removed_users
Expand Down
19 changes: 1 addition & 18 deletions server/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,6 @@
"""
import logging

try:
from psycogreen.gevent import patch_psycopg
except ImportError:
import sys
import traceback

exception_info = traceback.format_exc()
sys.stderr.write(
f"Failed to load required functions from the psycogreen library: { exception_info }\n"
)
sys.exit(1)

worker_class = "gevent"

workers = 2
Expand All @@ -59,12 +47,7 @@

max_requests_jitter = 5000


def do_post_fork(server, worker):
patch_psycopg()


post_fork = do_post_fork
timeout = 30


"""
Expand Down
27 changes: 23 additions & 4 deletions server/mergin/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
import os
import connexion
import wtforms_json
import gevent
from marshmallow import fields
from sqlalchemy.schema import MetaData
from flask_sqlalchemy import SQLAlchemy
from flask_marshmallow import Marshmallow
from flask import json, jsonify, request, abort, current_app, Flask
from flask import json, jsonify, request, abort, current_app, Flask, Request, Response
from flask_login import current_user, LoginManager
from flask_wtf.csrf import generate_csrf, CSRFProtect
from flask_migrate import Migrate
Expand All @@ -27,6 +28,7 @@
from typing import List, Dict, Optional

from .sync.utils import get_blacklisted_dirs, get_blacklisted_files
from .config import Configuration

convention = {
"ix": "ix_%(column_0_label)s",
Expand Down Expand Up @@ -105,9 +107,24 @@ def update_obj(self, obj):
field.populate_obj(obj, name)


def create_simple_app() -> Flask:
from .config import Configuration
class GeventTimeoutMiddleware:
"""Middleware to implement gevent.Timeout() for all requests"""

def __init__(self, app):
self.app = app

def __call__(self, environ, start_response):
request = Request(environ)
try:
with gevent.Timeout(Configuration.GEVENT_REQUEST_TIMEOUT):
return self.app(environ, start_response)
except gevent.Timeout:
logging.error(f"Gevent worker: Request {request.path} timed out")
resp = Response("Bad Gateway", mimetype="text/plain", status=502)
return resp(environ, start_response)


def create_simple_app() -> Flask:
app = connexion.FlaskApp(__name__, specification_dir=os.path.join(this_dir))
flask_app = app.app

Expand All @@ -117,6 +134,9 @@ def create_simple_app() -> Flask:
ma.init_app(flask_app)
Migrate(flask_app, db)
flask_app.connexion_app = app
# in case of gevent worker type use middleware to implement custom request timeout
if Configuration.GEVENT_WORKER:
flask_app.wsgi_app = GeventTimeoutMiddleware(flask_app.wsgi_app)

@flask_app.cli.command()
def init_db():
Expand All @@ -133,7 +153,6 @@ def init_db():
def create_app(public_keys: List[str] = None) -> Flask:
"""Factory function to create Flask app instance"""
from itsdangerous import BadTimeSignature, BadSignature
from .config import Configuration
from .auth import auth_required, decode_token
from .auth.models import User

Expand Down
6 changes: 6 additions & 0 deletions server/mergin/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ class Configuration(object):

# build hash number
BUILD_HASH = config("BUILD_HASH", default="")

# backend version
VERSION = config("VERSION", default=get_version())
SERVER_TYPE = config("SERVER_TYPE", default="ce")

# whether to run flask app with gevent worker type in gunicorn
# using gevent type of worker impose some requirements on code, e.g. to be greenlet safe, custom timeouts
GEVENT_WORKER = config("GEVENT_WORKER", default=False, cast=bool)
GEVENT_REQUEST_TIMEOUT = config("GEVENT_REQUEST_TIMEOUT", default=30, cast=int)
33 changes: 33 additions & 0 deletions server/mergin/tests/test_middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Copyright (C) Lutra Consulting Limited
#
# SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial

import gevent
import pytest

from ..app import create_simple_app, GeventTimeoutMiddleware
from ..config import Configuration


@pytest.mark.parametrize("use_middleware", [True, False])
def test_use_middleware(use_middleware):
"""Test using middleware"""
Configuration.GEVENT_WORKER = use_middleware
Configuration.GEVENT_REQUEST_TIMEOUT = 1
application = create_simple_app()

def ping():
gevent.sleep(Configuration.GEVENT_REQUEST_TIMEOUT + 1)
return "pong"

application.add_url_rule("/test", "ping", ping)
app_context = application.app_context()
app_context.push()

assert isinstance(application.wsgi_app, GeventTimeoutMiddleware) == use_middleware
# in case of gevent, dummy endpoint it set to time out
assert (
application.test_client().get("/test").status_code == 502
if use_middleware
else 200
)
Loading