-
Notifications
You must be signed in to change notification settings - Fork 123
feat: (opt-in): terminate handling of work when the request has already timed out #328
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
Merged
Merged
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
ccb4d90
feat: restore defaults present < 3.6.0, but retain customizability
jrmfg dcb6eb1
revert the test, too
jrmfg 0242768
also restore this assert :)
jrmfg 60611f5
feat: (opt-in): terminate handling of work when the request has alrea…
jrmfg 11284f0
fix tests
jrmfg 1821445
Merge branch 'main' into zombie-timeout
jrmfg af1f2fb
small test fixes
jrmfg 1e18b82
format
jrmfg 5d170a8
isort everything
jrmfg b143694
skip tests on mac
jrmfg fbffb7d
sort tuple of dicts in async tests before asserting
jrmfg 6f7d6c7
use double-quotes
jrmfg 25c6e7d
also skip tests on windows - this is all built for gunicorn, there's no
jrmfg 7a9acbe
skip import on windows
jrmfg 0a70fb9
easy stuff
jrmfg 7e559e5
add a few tests for sync worker timeouts
jrmfg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import logging | ||
| import time | ||
|
|
||
| import functions_framework | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| @functions_framework.http | ||
| def main(request): | ||
| timeout = 2 | ||
| for _ in range(timeout * 10): | ||
| time.sleep(0.1) | ||
| logger.info("logging message after timeout elapsed") | ||
| return "Hello, world!" | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import ctypes | ||
| import logging | ||
| import threading | ||
|
|
||
| from .exceptions import RequestTimeoutException | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class ThreadingTimeout(object): # pragma: no cover | ||
| def __init__(self, seconds): | ||
| self.seconds = seconds | ||
| self.target_tid = threading.current_thread().ident | ||
| self.timer = None | ||
|
|
||
| def __enter__(self): | ||
| self.timer = threading.Timer(self.seconds, self._raise_exc) | ||
| self.timer.start() | ||
| return self | ||
|
|
||
| def __exit__(self, exc_type, exc_val, exc_tb): | ||
| self.timer.cancel() | ||
| if exc_type is RequestTimeoutException: | ||
| logger.warning( | ||
| "Request handling exceeded {0} seconds timeout; terminating request handling...".format( | ||
| self.seconds | ||
| ), | ||
| exc_info=(exc_type, exc_val, exc_tb), | ||
| ) | ||
| return False | ||
|
|
||
| def _raise_exc(self): | ||
| ret = ctypes.pythonapi.PyThreadState_SetAsyncExc( | ||
| ctypes.c_long(self.target_tid), ctypes.py_object(RequestTimeoutException) | ||
| ) | ||
| if ret == 0: | ||
| raise ValueError("Invalid thread ID {}".format(self.target_tid)) | ||
| elif ret > 1: | ||
| ctypes.pythonapi.PyThreadState_SetAsyncExc( | ||
| ctypes.c_long(self.target_tid), None | ||
| ) | ||
| raise SystemError("PyThreadState_SetAsyncExc failed") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import logging | ||
| import time | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def function(request): | ||
| # sleep for 1200 total ms (1.8 sec) | ||
jrmfg marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| for _ in range(12): | ||
| time.sleep(0.1) | ||
| logger.info("some extra logging message") | ||
| return "success", 200 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,198 @@ | ||
| # Copyright 2024 Google LLC | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| import pathlib | ||
| import socket | ||
| import time | ||
|
|
||
| from multiprocessing import Process | ||
|
|
||
| import pytest | ||
| import requests | ||
|
|
||
| ff_gunicorn = pytest.importorskip("functions_framework._http.gunicorn") | ||
|
|
||
|
|
||
| from functions_framework import create_app | ||
|
|
||
| TEST_FUNCTIONS_DIR = pathlib.Path(__file__).resolve().parent / "test_functions" | ||
| TEST_HOST = "0.0.0.0" | ||
| TEST_PORT = "8080" | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def run_around_tests(): | ||
| # the test samples test also listens on 8080, so let's be good stewards of | ||
| # the port and make sure it's free | ||
| _wait_for_no_listen(TEST_HOST, TEST_PORT) | ||
| yield | ||
| _wait_for_no_listen(TEST_HOST, TEST_PORT) | ||
|
|
||
|
|
||
| @pytest.mark.skipif("platform.system() == 'Windows'") | ||
| @pytest.mark.skipif("platform.system() == 'Darwin'") | ||
| @pytest.mark.slow_integration_test | ||
| def test_no_timeout_allows_request_processing_to_finish(): | ||
| source = TEST_FUNCTIONS_DIR / "timeout" / "main.py" | ||
| target = "function" | ||
|
|
||
| app = create_app(target, source) | ||
|
|
||
| options = {} | ||
|
|
||
| gunicorn_app = ff_gunicorn.GunicornApplication( | ||
| app, TEST_HOST, TEST_PORT, False, **options | ||
| ) | ||
|
|
||
| gunicorn_p = Process(target=gunicorn_app.run) | ||
| gunicorn_p.start() | ||
|
|
||
| _wait_for_listen(TEST_HOST, TEST_PORT) | ||
|
|
||
| result = requests.get("http://{}:{}/".format(TEST_HOST, TEST_PORT)) | ||
|
|
||
| gunicorn_p.terminate() | ||
| gunicorn_p.join() | ||
|
|
||
| assert result.status_code == 200 | ||
|
|
||
|
|
||
| @pytest.mark.skipif("platform.system() == 'Windows'") | ||
| @pytest.mark.skipif("platform.system() == 'Darwin'") | ||
| @pytest.mark.slow_integration_test | ||
| def test_timeout_but_not_threaded_timeout_enabled_does_not_kill(monkeypatch): | ||
| monkeypatch.setenv("CLOUD_RUN_TIMEOUT_SECONDS", "1") | ||
| monkeypatch.setenv("THREADED_TIMEOUT_ENABLED", "false") | ||
jrmfg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| source = TEST_FUNCTIONS_DIR / "timeout" / "main.py" | ||
| target = "function" | ||
|
|
||
| app = create_app(target, source) | ||
|
|
||
| options = {} | ||
|
|
||
| gunicorn_app = ff_gunicorn.GunicornApplication( | ||
| app, TEST_HOST, TEST_PORT, False, **options | ||
| ) | ||
|
|
||
| gunicorn_p = Process(target=gunicorn_app.run) | ||
| gunicorn_p.start() | ||
|
|
||
| _wait_for_listen(TEST_HOST, TEST_PORT) | ||
|
|
||
| result = requests.get("http://{}:{}/".format(TEST_HOST, TEST_PORT)) | ||
|
|
||
| gunicorn_p.terminate() | ||
| gunicorn_p.join() | ||
|
|
||
| assert result.status_code == 200 | ||
|
|
||
|
|
||
| @pytest.mark.skipif("platform.system() == 'Windows'") | ||
| @pytest.mark.skipif("platform.system() == 'Darwin'") | ||
| @pytest.mark.slow_integration_test | ||
| def test_timeout_and_threaded_timeout_enabled_kills(monkeypatch): | ||
| monkeypatch.setenv("CLOUD_RUN_TIMEOUT_SECONDS", "1") | ||
| monkeypatch.setenv("THREADED_TIMEOUT_ENABLED", "true") | ||
| source = TEST_FUNCTIONS_DIR / "timeout" / "main.py" | ||
| target = "function" | ||
|
|
||
| app = create_app(target, source) | ||
|
|
||
| options = {} | ||
|
|
||
| gunicorn_app = ff_gunicorn.GunicornApplication( | ||
| app, TEST_HOST, TEST_PORT, False, **options | ||
| ) | ||
|
|
||
| gunicorn_p = Process(target=gunicorn_app.run) | ||
| gunicorn_p.start() | ||
|
|
||
| _wait_for_listen(TEST_HOST, TEST_PORT) | ||
|
|
||
| result = requests.get("http://{}:{}/".format(TEST_HOST, TEST_PORT)) | ||
|
|
||
| gunicorn_p.terminate() | ||
| gunicorn_p.join() | ||
|
|
||
| # Any exception raised in execution is a 500 error. Cloud Functions 1st gen and | ||
| # 2nd gen/Cloud Run infrastructure doing the timeout will return a 408 (gen 1) | ||
| # or 504 (gen 2/CR) at the infrastructure layer when request timeouts happen, | ||
| # and this code will only be available to the user in logs. | ||
| assert result.status_code == 500 | ||
|
|
||
|
|
||
| @pytest.mark.skipif("platform.system() == 'Windows'") | ||
| @pytest.mark.skipif("platform.system() == 'Darwin'") | ||
| @pytest.mark.slow_integration_test | ||
| def test_timeout_and_threaded_timeout_enabled_but_timeout_not_exceeded_doesnt_kill( | ||
| monkeypatch, | ||
| ): | ||
| monkeypatch.setenv("CLOUD_RUN_TIMEOUT_SECONDS", "2") | ||
| monkeypatch.setenv("THREADED_TIMEOUT_ENABLED", "true") | ||
| source = TEST_FUNCTIONS_DIR / "timeout" / "main.py" | ||
| target = "function" | ||
|
|
||
| app = create_app(target, source) | ||
|
|
||
| options = {} | ||
|
|
||
| gunicorn_app = ff_gunicorn.GunicornApplication( | ||
| app, TEST_HOST, TEST_PORT, False, **options | ||
| ) | ||
|
|
||
| gunicorn_p = Process(target=gunicorn_app.run) | ||
| gunicorn_p.start() | ||
|
|
||
| _wait_for_listen(TEST_HOST, TEST_PORT) | ||
|
|
||
| result = requests.get("http://{}:{}/".format(TEST_HOST, TEST_PORT)) | ||
|
|
||
| gunicorn_p.terminate() | ||
| gunicorn_p.join() | ||
|
|
||
| assert result.status_code == 200 | ||
|
|
||
|
|
||
| @pytest.mark.skip | ||
| def _wait_for_listen(host, port, timeout=10): | ||
| # Used in tests to make sure that the gunicorn app has booted and is | ||
| # listening before sending a test request | ||
| start_time = time.perf_counter() | ||
| while True: | ||
| try: | ||
| with socket.create_connection((host, port), timeout=timeout): | ||
| break | ||
| except OSError as ex: | ||
| time.sleep(0.01) | ||
| if time.perf_counter() - start_time >= timeout: | ||
| raise TimeoutError( | ||
| "Waited too long for port {} on host {} to start accepting " | ||
| "connections.".format(port, host) | ||
| ) from ex | ||
|
|
||
|
|
||
| @pytest.mark.skip | ||
| def _wait_for_no_listen(host, port, timeout=10): | ||
| # Used in tests to make sure that the | ||
jrmfg marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| start_time = time.perf_counter() | ||
| while True: | ||
| try: | ||
| with socket.create_connection((host, port), timeout=timeout): | ||
| time.sleep(0.01) | ||
| if time.perf_counter() - start_time >= timeout: | ||
| raise TimeoutError( | ||
| "Waited too long for port {} on host {} to stop accepting " | ||
| "connections.".format(port, host) | ||
| ) | ||
| except OSError as ex: | ||
| break | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.