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

Gunicorn with UvicornWorker is not respecting the timeout #611

Closed
assem-ch opened this issue Apr 6, 2020 · 17 comments · Fixed by #2302
Closed

Gunicorn with UvicornWorker is not respecting the timeout #611

assem-ch opened this issue Apr 6, 2020 · 17 comments · Fixed by #2302

Comments

@assem-ch
Copy link

assem-ch commented Apr 6, 2020

I am setting up a timeout check so I made and endpoint:

@app.get("/tc", status_code=200)
def timeout_check():
    time.sleep(500)
    return "NOT OK"

I am using the docker image tiangolo/uvicorn-gunicorn-fastapi:python3.7
and my command to run the server:

CMD ["gunicorn","--log-level","debug","--keep-alive","15", "--reload", "-b", "0.0.0.0:8080", "--timeout", "15", "--worker-class=uvicorn.workers.UvicornH11Worker", "--workers=10", "myapp.main:app"]

I am expecting the endpoint to fail after 15 seconds, but it doesn't. Seems like the timeout is not respected.

Is there any mistake in my configuration?

Important

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@euri10
Copy link
Member

euri10 commented Apr 6, 2020

there's probably an error in the way the gunicorn config for the timeout is passed down to the UvicornWorker ie
"timeout_notify": self.timeout should certainly be "timeout_notify": self.cfg.timeout

That said I tested it and having an endpoint that sleep more than this timeout does kill the gunicorn worker

@assem-ch
Copy link
Author

assem-ch commented Apr 7, 2020

@euri10 I am stuck testing to get a timeout , can you please give me your test example, or point me if I have something missing in my configuration.

@euri10
Copy link
Member

euri10 commented Apr 7, 2020

it seems like I typed too fast I'm sorry @assem-ch , I meant it does not kill the timeout so I'm afraid I can only confirm what's happening to you, even after I fixed passing the gunicorn timeout correctly like described above.

timeout_notify seems to be only used to tell the worker it's alive and well, but nothing in the response cycle enforces a "short circuit" as far as I understood.

@euri10
Copy link
Member

euri10 commented Apr 7, 2020

this said, reading gunicorn docs here

For the non sync workers it just means that the worker process is still communicating and is not tied to the length of time required to handle a single request.

it seems intended if I get it correctly ?

@assem-ch
Copy link
Author

assem-ch commented Apr 7, 2020

I got that the timeout_notify is for non responsive worker only, if a worker is responding, it will not be killed. Is there a way to kill workers with a timeout even if they are responsive? I am not sure this question should be asked here or on Gunicorn.

@euri10
Copy link
Member

euri10 commented Apr 7, 2020

benoitc/gunicorn#1493 is an interesting read

@assem-ch
Copy link
Author

I went through that thread and I got to this conclusion:
For sync workers , worker timeout and request timeout are the same since the worker will blocked doing one request. This is not the case for uvicorn. my aim is request timeout, so killing the worker wont work except if a worker can be limited to one request. so:

  • can we do the timeout per request for uvicorn?
  • or can we limit the worker to only one request or make uvicorn behave as wsgi ?

@euri10
Copy link
Member

euri10 commented Apr 12, 2020

for option 2 , untested and on top of my head, I think you can

  1. use the max_requests flag which will be passed to uvicorn's config (should you want to use gunicorn on top of uvicorn, don't know if it's a prerequisite for you)

  2. if you only use uvicorn use the limit_max_requests flag

@assem-ch
Copy link
Author

I will try that.

Using Gunicorn based on fastapi recommendation for prod configuration, we have high traffic apis , do you think we can use uvicorn alone for that?

@assem-ch
Copy link
Author

is limit_max_requests global for all workers or per worker?

@assem-ch
Copy link
Author

@euri10 I was playing around the uvicorn worker to implement a woker that do only one request and timeout it if takes long.

class ShortLifeWorker(UvicornWorker):
    # --max-requests = 1
    # --timeout = 15
    # --workers=10
    def notify(self):
        pass

The issue with this is that even passive workers with no request to process are timeout, any suggestion to know if a worker is passive or not?

@seekingpeace
Copy link

seekingpeace commented Jun 27, 2020

For anyone who wants to restart the worker process if a function is taking too long and he is sure that restarting the worker will be a quick fix and will work, based on https://code-maven.com/python-timeout we can interrupt the code raise exception and then raise signal.SIGINT, as soon as worker process is killed, gunicorn detects it and spawns a new worker in it's place
example:

from fastapi import FastAPI
from fastapi.testclient import TestClient
import time
import signal
import os
import logger

app = FastAPI()

class TimeOutException(Exception):
   pass

def alarm_handler(signum, frame):
    print("ALARM signal received")
    pid = os.getpid()
    os.kill(pid,signal.SIGINT)
    raise TimeOutException()

signal.signal(signal.SIGALRM, alarm_handler)

@app.get("/ping")
def read_main():
    signal.alarm(8)
    try:
        time.sleep(5)
        print("till here is fine")
        time.sleep(5)
    except TimeOutException as ex:
        print(ex)
    signal.alarm(0)   
    return {"msg": "Hello World"}

gunicorn test_fastapi:app -w 2 -k uvicorn.workers.UvicornWorker -b 0.0.0.0:8007

@assem-ch
Copy link
Author

@seekingpeace consider that a worker can serve many requests so if one request killed the worker , other requests will get lost. I think your solution will work only if you limit one request by worker.

@seekingpeace
Copy link

seekingpeace commented Jul 8, 2020

@assem-ch

pid = os.getpid()
os.kill(pid,signal.SIGINT)

these lines can be commented and you can get a function level timeout, so that instead of killing the process, you save it. Also, as per gunicorn, you are still left with 0.1 sec before server gets killed from sigint

@assem-ch
Copy link
Author

I end up with this solution:

class RequestKillerWorker(UvicornWorker):
  
    def notify(self):
        if not self.server.server_state.tasks:
            self.tmp.notify()

so basically the worker is killed only if busy after the timeout

@Kludex Kludex added this to the Version 0.22.0 milestone Nov 1, 2022
@winstxnhdw
Copy link

winstxnhdw commented Feb 27, 2024

Uvicorn is respecting Gunicorn's timeout config just fine. We can probably close this.

# pylint: skip-file

from server import initialise
from server.config import Config

wsgi_app = f'{initialise.__module__}:{initialise.__name__}()'
reload = False
accesslog = '-'
preload_app = True
bind = f'0.0.0.0:{Config.server_port}'
workers = Config.worker_count
worker_class = 'uvicorn.workers.UvicornWorker'
timeout = 30
[2024-02-27 19:51:02 +0000] [7] [INFO] Starting gunicorn 21.2.0
[2024-02-27 19:51:02 +0000] [7] [INFO] Listening at: http://0.0.0.0:5000 (7)
[2024-02-27 19:51:02 +0000] [7] [INFO] Using worker: uvicorn.workers.UvicornWorker
[2024-02-27 19:51:02 +0000] [35] [INFO] Booting worker with pid: 35
[2024-02-27 19:51:02 +0000] [35] [INFO] Started server process [35]
[2024-02-27 19:51:02 +0000] [35] [INFO] Waiting for application startup.
Fetching 9 files:  11%|█         | 1/9 [00:00<00:05,  1.40it/s]2024-02-27 19:51:10,970 INFO success: server entered RUNNING state, process has stayed up for > than 10 seconds (startsecs)
2024-02-27 19:51:10,971 INFO success: caddy entered RUNNING state, process has stayed up for > than 10 seconds (startsecs)
[2024-02-27 19:51:12 +0000] [7] [CRITICAL] WORKER TIMEOUT (pid:35)
[2024-02-27 19:51:13 +0000] [7] [ERROR] Worker (pid:35) was sent SIGKILL! Perhaps out of memory?
[2024-02-27 19:51:13 +0000] [46] [INFO] Booting worker with pid: 46
[2024-02-27 19:51:13 +0000] [46] [INFO] Started server process [46]
[2024-02-27 19:51:13 +0000] [46] [INFO] Waiting for application startup.
Fetching 9 files:   0%|          | 0/9 [00:00<?, ?it/s]

@Kludex Kludex removed this from the Version 0.22.0 milestone Apr 13, 2024
@Kludex
Copy link
Sponsor Member

Kludex commented Apr 13, 2024

We are deprecating the workers within Uvicorn, feel free to create this issue on https://github.com/Kludex/uvicorn-worker.

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

Successfully merging a pull request may close this issue.

5 participants