Skip to content

Commit

Permalink
Proposed as a solution for issue benoitc#1658
Browse files Browse the repository at this point in the history
A new timeout-start-after value is added to config, granting workers an
initial grace period to do time-consuming initialization, after which
they are held to timeout stricture.

Previous attempts of mine to form an acceptable pull request have failed
linting in CI, all in code I didn't touch. This branch is newly minted
from the current upstream master.
  • Loading branch information
Richard Winslow committed Jun 1, 2020
1 parent 4bed9e7 commit 565b8a4
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 1 deletion.
17 changes: 17 additions & 0 deletions docs/source/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1363,6 +1363,23 @@ 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.

.. _timeout-start-after:

timeout_start_after
~~~~~~~~~~~~~~~~~~~

* ``--timeout-start-after INT``
* ``0``

Wait this many seconds before enforcing timeout on a worker.

Some workers may take a long time to initialize, even though they are
expected to be highly available once they're ready. Such a worker should
have a small timeout setting, but Gunicorn needs a way to "forgive" the
long delay during initialization. When non-zero (zero is the default),
Gunicorn waits this many seconds after a worker is created before
enforcing timeout.

.. _graceful-timeout:

graceful_timeout
Expand Down
5 changes: 4 additions & 1 deletion gunicorn/arbiter.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def setup(self, app):
self.address = self.cfg.address
self.num_workers = self.cfg.workers
self.timeout = self.cfg.timeout
self.timeout_start_after = self.cfg.timeout_start_after
self.proc_name = self.cfg.proc_name

self.log.debug('Current configuration:\n{0}'.format(
Expand Down Expand Up @@ -492,7 +493,9 @@ def murder_workers(self):
workers = list(self.WORKERS.items())
for (pid, worker) in workers:
try:
if time.time() - worker.tmp.last_update() <= self.timeout:
now = time.time()
grace = now - worker.created < self.timeout_start_after
if grace or now - worker.tmp.last_update() <= self.timeout:
continue
except (OSError, ValueError):
continue
Expand Down
20 changes: 20 additions & 0 deletions gunicorn/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,26 @@ class Timeout(Setting):
"""


class TimeoutStartAfter(Setting):
name = "timeout_start_after"
section = "Worker Processes"
cli = ["--timeout-start-after"]
meta = "INT"
validator = validate_pos_int
type = int
default = 0
desc = """\
Wait this many seconds before enforcing timeout on a worker.
Some workers may take a long time to initialize, even though they are
expected to be highly available once they're ready. Such a worker should
have a small timeout setting, but Gunicorn needs a way to "forgive" the
long delay during initialization. When non-zero (zero is the default),
Gunicorn waits this many seconds after a worker is created before
enforcing timeout.
"""


class GracefulTimeout(Setting):
name = "graceful_timeout"
section = "Worker Processes"
Expand Down
1 change: 1 addition & 0 deletions gunicorn/workers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def __init__(self, age, ppid, sockets, app, timeout, cfg, log):
self.booted = False
self.aborted = False
self.reloader = None
self.created = time.time()

self.nr = 0

Expand Down
47 changes: 47 additions & 0 deletions tests/test_arbiter.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# See the NOTICE for more information.

import os
import signal
import unittest.mock as mock

import gunicorn.app.base
Expand Down Expand Up @@ -148,6 +149,52 @@ def test_arbiter_reap_workers(mock_os_waitpid):
arbiter.cfg.child_exit.assert_called_with(arbiter, mock_worker)


@mock.patch('os.kill')
@mock.patch('time.time')
def test_arbiter_murder_workers(mock_time, mock_kill):
mock_time.side_effect = [1000.0]

mock_worker = mock.Mock()
mock_last_update = mock.Mock()
mock_last_update.side_effect = [998.0]
mock_worker.tmp.last_update = mock_last_update
mock_worker.aborted = False
mock_worker.created = 1.0

arbiter = gunicorn.arbiter.Arbiter(DummyApplication())
arbiter.log = mock.Mock()
arbiter.timeout = 1
arbiter.timeout_start_after = 0
arbiter.WORKERS = {42: mock_worker}

arbiter.murder_workers()

mock_worker.tmp.last_update.assert_called_with()
mock_kill.assert_called_with(42, signal.SIGABRT)


@mock.patch('os.kill')
@mock.patch('time.time')
def test_arbiter_respects_timeout_start_after(mock_time, mock_kill):
mock_time.side_effect = [1000.0]

mock_worker = mock.Mock()
mock_last_update = mock.Mock()
mock_worker.tmp.last_update = mock_last_update
mock_last_update.side_effect = [991.0]
mock_worker.created = 991.0

arbiter = gunicorn.arbiter.Arbiter(DummyApplication())
arbiter.timeout = 1
arbiter.timeout_start_after = 10
arbiter.WORKERS = {42: mock_worker}

arbiter.murder_workers()

mock_worker.tmp.last_update.assert_not_called()
mock_kill.assert_not_called()


class PreloadedAppWithEnvSettings(DummyApplication):
"""
Simple application that makes use of the 'preload' feature to
Expand Down

0 comments on commit 565b8a4

Please sign in to comment.