-
Notifications
You must be signed in to change notification settings - Fork 192
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Close SQLA session after every REST API request
This is needed due to the server running in threaded mode, i.e., creating a new thread for each incoming request. This concept is great for handling many requests, but crashes when used together with AiiDA's global singleton SQLA session used, no matter the backend of the profile by the `QueryBuilder`. Specifically, this leads to issues with the SQLA QueuePool, since the connections are not properly released when a thread is closed. This leads to unintended QueuePool overflow. This fix wraps all HTTP method requests and makes sure to close the current thread's SQLA session after the request as been completely handled. Use Flask-RESTful's integrated `Resource` attribute `method_decorators` to apply `close_session` wrapper to all and any HTTP request that may be requested of AiiDA's `BaseResource` (and its sub-classes). Additionally, remove the `__init__` function overwritten in `Node(BaseResource)`, since it is redundant, and the attributes `tclass` is not relevant with v4 (AiiDA v1.0.0 and above), but was never removed. It should have been removed when moving to v4 in 4ff2829. Concerning the added tests: the timeout needs to be set for Python 3.5 in order to stop the http socket and properly raise (and escape out of an infinite loop). The `capfd` fixture must be used, otherwise the exception cannot be properly captured. The tests were simplified into the pytest scheme with ideas from @sphuber and @greschd.
- Loading branch information
Showing
9 changed files
with
212 additions
and
33 deletions.
There are no files selected for viewing
This file contains 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 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 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 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 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 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 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,50 @@ | ||
| """pytest fixtures for use with the aiida.restapi tests""" | ||
| import pytest | ||
|
|
||
|
|
||
| @pytest.fixture(scope='function') | ||
| def restapi_server(): | ||
| """Make REST API server""" | ||
| from werkzeug.serving import make_server | ||
|
|
||
| from aiida.restapi.common.config import CLI_DEFAULTS | ||
| from aiida.restapi.run_api import configure_api | ||
|
|
||
| def _restapi_server(restapi=None): | ||
| if restapi is None: | ||
| flask_restapi = configure_api() | ||
| else: | ||
| flask_restapi = configure_api(flask_api=restapi) | ||
|
|
||
| return make_server( | ||
| host=CLI_DEFAULTS['HOST_NAME'], | ||
| port=int(CLI_DEFAULTS['PORT']), | ||
| app=flask_restapi.app, | ||
| threaded=True, | ||
| processes=1, | ||
| request_handler=None, | ||
| passthrough_errors=True, | ||
| ssl_context=None, | ||
| fd=None | ||
| ) | ||
|
|
||
| return _restapi_server | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def server_url(): | ||
| from aiida.restapi.common.config import CLI_DEFAULTS, API_CONFIG | ||
|
|
||
| return 'http://{hostname}:{port}{api}'.format( | ||
| hostname=CLI_DEFAULTS['HOST_NAME'], port=CLI_DEFAULTS['PORT'], api=API_CONFIG['PREFIX'] | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def restrict_sqlalchemy_queuepool(aiida_profile): | ||
| """Create special SQLAlchemy engine for use with QueryBuilder - backend-agnostic""" | ||
| from aiida.manage.manager import get_manager | ||
|
|
||
| backend_manager = get_manager().get_backend_manager() | ||
| backend_manager.reset_backend_environment() | ||
| backend_manager.load_backend_environment(aiida_profile, pool_timeout=1, max_overflow=0) |
This file contains 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 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,121 @@ | ||
| # -*- coding: utf-8 -*- | ||
| ########################################################################### | ||
| # Copyright (c), The AiiDA team. All rights reserved. # | ||
| # This file is part of the AiiDA code. # | ||
| # # | ||
| # The code is hosted on GitHub at https://github.com/aiidateam/aiida-core # | ||
| # For further information on the license, see the LICENSE.txt file # | ||
| # For further information please visit http://www.aiida.net # | ||
| ########################################################################### | ||
| """Tests for the `aiida.restapi` module, using it in threaded mode. | ||
| Threaded mode is the default (and only) way to run the AiiDA REST API (see `aiida.restapi.run_api:run_api()`). | ||
| This test file's layout is inspired by https://gist.github.com/prschmid/4643738 | ||
| """ | ||
| import time | ||
| from threading import Thread | ||
|
|
||
| import requests | ||
| import pytest | ||
|
|
||
| NO_OF_REQUESTS = 100 | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures('clear_database_before_test', 'restrict_sqlalchemy_queuepool') | ||
| def test_run_threaded_server(restapi_server, server_url, aiida_localhost): | ||
| """Run AiiDA REST API threaded in a separate thread and perform many sequential requests""" | ||
|
|
||
| server = restapi_server() | ||
| computer_id = aiida_localhost.uuid | ||
|
|
||
| # Create a thread that will contain the running server, | ||
| # since we do not wish to block the main thread | ||
| server_thread = Thread(target=server.serve_forever) | ||
|
|
||
| try: | ||
| server_thread.start() | ||
|
|
||
| for _ in range(NO_OF_REQUESTS): | ||
| response = requests.get(server_url + '/computers/{}'.format(computer_id), timeout=10) | ||
|
|
||
| assert response.status_code == 200 | ||
|
|
||
| try: | ||
| response_json = response.json() | ||
| except ValueError: | ||
| pytest.fail('Could not turn response into JSON. Response: {}'.format(response.raw)) | ||
| else: | ||
| assert 'data' in response_json | ||
|
|
||
| except Exception as exc: # pylint: disable=broad-except | ||
| pytest.fail('Something went terribly wrong! Exception: {}'.format(repr(exc))) | ||
| finally: | ||
| server.shutdown() | ||
|
|
||
| # Wait a total of 1 min (100 x 0.6 s) for the Thread to close/join, else fail | ||
| for _ in range(100): | ||
| if server_thread.is_alive(): | ||
| time.sleep(0.6) | ||
| else: | ||
| break | ||
| else: | ||
| pytest.fail('Thread did not close/join within 1 min after REST API server was called to shutdown') | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures('clear_database_before_test', 'restrict_sqlalchemy_queuepool') | ||
| def test_run_without_close_session(restapi_server, server_url, aiida_localhost, capfd): | ||
| """Run AiiDA REST API threaded in a separate thread and perform many sequential requests""" | ||
| from aiida.restapi.api import AiidaApi | ||
| from aiida.restapi.resources import Computer | ||
|
|
||
| class NoCloseSessionApi(AiidaApi): | ||
| """Add Computer to this API (again) with a new endpoint, but pass an empty list for `get_decorators`""" | ||
|
|
||
| def __init__(self, app=None, **kwargs): | ||
| super().__init__(app=app, **kwargs) | ||
|
|
||
| # This is a copy of adding the `Computer` resource, | ||
| # but only a few URLs are added, and `get_decorators` is passed with an empty list. | ||
| extra_kwargs = kwargs.copy() | ||
| extra_kwargs.update({'get_decorators': []}) | ||
| self.add_resource( | ||
| Computer, | ||
| '/computers_no_close_session/', | ||
| '/computers_no_close_session/<id>/', | ||
| endpoint='computers_no_close_session', | ||
| strict_slashes=False, | ||
| resource_class_kwargs=extra_kwargs, | ||
| ) | ||
|
|
||
| server = restapi_server(NoCloseSessionApi) | ||
| computer_id = aiida_localhost.uuid | ||
|
|
||
| # Create a thread that will contain the running server, | ||
| # since we do not wish to block the main thread | ||
| server_thread = Thread(target=server.serve_forever) | ||
|
|
||
| try: | ||
| server_thread.start() | ||
|
|
||
| for _ in range(NO_OF_REQUESTS): | ||
| requests.get(server_url + '/computers_no_close_session/{}'.format(computer_id), timeout=10) | ||
| pytest.fail('{} requests were not enough to raise a SQLAlchemy TimeoutError!'.format(NO_OF_REQUESTS)) | ||
|
|
||
| except (requests.exceptions.ConnectionError, OSError): | ||
| pass | ||
| except Exception as exc: # pylint: disable=broad-except | ||
| pytest.fail('Something went terribly wrong! Exception: {}'.format(repr(exc))) | ||
| finally: | ||
| server.shutdown() | ||
|
|
||
| # Wait a total of 1 min (100 x 0.6 s) for the Thread to close/join, else fail | ||
| for _ in range(100): | ||
| if server_thread.is_alive(): | ||
| time.sleep(0.6) | ||
| else: | ||
| break | ||
| else: | ||
| pytest.fail('Thread did not close/join within 1 min after REST API server was called to shutdown') | ||
|
|
||
| captured = capfd.readouterr() | ||
| assert 'sqlalchemy.exc.TimeoutError: QueuePool limit of size ' in captured.err |