From 5c9efa9d746f53c7173c92b6920651c118c78ee1 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Fri, 30 Mar 2018 22:08:48 +0100 Subject: [PATCH] Enable multiple threads, disabled muliple workers This sets multiple threads in Deckhand's chart config (4) and set workers to just 1. Deckhand's database is not configured to work with multiprocessing. Currently there is a data race on acquiring shared SQLAlchemy engine pooled connection strings when workers > 1. As a workaround, we use multiple threads but only 1 worker. For more information, see: https://github.com/att-comdev/deckhand/issues/20 Change-Id: I60adeffff5461fdda957124232bc5a606baae413 --- charts/deckhand/templates/deployment.yaml | 9 ++++++-- charts/deckhand/values.yaml | 9 ++++++-- deckhand/control/api.py | 2 +- docs/source/getting-started.rst | 6 +++--- entrypoint.sh | 13 ++++++++++-- tools/functional-tests.sh | 26 +++++++++++------------ 6 files changed, 41 insertions(+), 24 deletions(-) diff --git a/charts/deckhand/templates/deployment.yaml b/charts/deckhand/templates/deployment.yaml index abd4bb00..592f3c08 100644 --- a/charts/deckhand/templates/deployment.yaml +++ b/charts/deckhand/templates/deployment.yaml @@ -47,10 +47,15 @@ spec: env: - name: 'DECKHAND_API_TIMEOUT' value: {{ .Values.conf.uwsgi.timeout | default 600 | quote }} + # NOTE(fmontei): Deckhand's database is not configured to work with + # multiprocessing. Currently there is a data race on acquiring shared + # SQLAlchemy engine pooled connection strings when workers > 1. As a + # workaround, we use multiple threads but only 1 worker. For more + # information, see: https://github.com/att-comdev/deckhand/issues/20 - name: 'DECKHAND_API_WORKERS' - value: {{ .Values.conf.uwsgi.workers | default 4 | quote }} + value: {{ .Values.conf.uwsgi.workers | default 1 | quote }} - name: 'DECKHAND_API_THREADS' - value: {{ .Values.conf.uwsgi.threads | default 1 | quote }} + value: {{ .Values.conf.uwsgi.threads | default 4 | quote }} image: {{ .Values.images.tags.deckhand }} imagePullPolicy: {{ .Values.images.pull_policy }} {{ tuple $envAll $envAll.Values.pod.resources.api | include "helm-toolkit.snippets.kubernetes_resources" | indent 10 }} diff --git a/charts/deckhand/values.yaml b/charts/deckhand/values.yaml index d85a27ef..8c6bc828 100644 --- a/charts/deckhand/values.yaml +++ b/charts/deckhand/values.yaml @@ -179,8 +179,13 @@ secrets: conf: uwsgi: - threads: 1 - workers: 4 + # NOTE(fmontei): Deckhand's database is not configured to work with + # multiprocessing. Currently there is a data race on acquiring shared + # SQLAlchemy engine pooled connection strings when workers > 1. As a + # workaround, we use multiple threads but only 1 worker. For more + # information, see: https://github.com/att-comdev/deckhand/issues/20 + threads: 4 + workers: 1 policy: admin_api: role:admin deckhand:create_cleartext_documents: rule:admin_api diff --git a/deckhand/control/api.py b/deckhand/control/api.py index 00bf6f06..d8d2cf16 100644 --- a/deckhand/control/api.py +++ b/deckhand/control/api.py @@ -33,7 +33,7 @@ CONFIG_FILES = ['deckhand.conf', 'deckhand-paste.ini'] def _get_config_files(env=None): if env is None: env = os.environ - dirname = env.get('OS_DECKHAND_CONFIG_DIR', '/etc/deckhand').strip() + dirname = env.get('DECKHAND_CONFIG_DIR', '/etc/deckhand').strip() return [os.path.join(dirname, config_file) for config_file in CONFIG_FILES] diff --git a/docs/source/getting-started.rst b/docs/source/getting-started.rst index af28ffea..bd8ee81b 100644 --- a/docs/source/getting-started.rst +++ b/docs/source/getting-started.rst @@ -139,8 +139,8 @@ Create the directory ``/etc/deckhand`` and copy the config file there:: To specify an alternative directory for the config file, run:: - $ export OS_DECKHAND_CONFIG_DIR= - $ [sudo] cp etc/deckhand/deckhand.conf.sample ${OS_DECKHAND_CONFIG_DIR}/deckhand.conf + $ export DECKHAND_CONFIG_DIR= + $ [sudo] cp etc/deckhand/deckhand.conf.sample ${DECKHAND_CONFIG_DIR}/deckhand.conf To conveniently create an ephemeral PostgreSQL DB run:: @@ -152,7 +152,7 @@ Retrieve the environment variable which contains connection information:: declare -x PIFPAF_POSTGRESQL_URL="postgresql://localhost/postgres?host=/tmp/tmpsg6tn3l9&port=9824" Substitute the connection information into the config file in -``${OS_DECKHAND_CONFIG_DIR}``:: +``${DECKHAND_CONFIG_DIR}``:: [database] diff --git a/entrypoint.sh b/entrypoint.sh index debce75e..20201301 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -20,10 +20,19 @@ set -ex PORT=${PORT:-9000} # How long uWSGI should wait for each deckhand response DECKHAND_API_TIMEOUT=${DECKHAND_API_TIMEOUT:-"600"} + +# NOTE(fmontei): Deckhand's database is not configured to work with +# multiprocessing. Currently there is a data race on acquiring shared +# SQLAlchemy engine pooled connection strings when workers > 1. As a +# workaround, we use multiple threads but only 1 worker. For more +# information, see: https://github.com/att-comdev/deckhand/issues/20 + # Number of uWSGI workers to handle API requests -DECKHAND_API_WORKERS=${DECKHAND_API_WORKERS:-"4"} +DECKHAND_API_WORKERS=${DECKHAND_API_WORKERS:-"1"} # Threads per worker -DECKHAND_API_THREADS=${DECKHAND_API_THREADS:-"1"} +DECKHAND_API_THREADS=${DECKHAND_API_THREADS:-"4"} +# The Deckhand configuration directory containing deckhand.conf +DECKHAND_CONFIG_DIR=${DECKHAND_CONFIG_DIR:-"/etc/deckhand/deckhand.conf"} # Start deckhand application exec uwsgi \ diff --git a/tools/functional-tests.sh b/tools/functional-tests.sh index 3e5c8389..2d7c7be6 100755 --- a/tools/functional-tests.sh +++ b/tools/functional-tests.sh @@ -68,7 +68,7 @@ function gen_config { export DECKHAND_TEST_URL=http://localhost:9000 export DATABASE_URL=postgresql+psycopg2://deckhand:password@$POSTGRES_IP:5432/deckhand # Used by Deckhand's initialization script to search for config files. - export OS_DECKHAND_CONFIG_DIR=$CONF_DIR + export DECKHAND_CONFIG_DIR=$CONF_DIR cp etc/deckhand/logging.conf.sample $CONF_DIR/logging.conf @@ -198,19 +198,19 @@ gen_config gen_paste gen_policy +ROOTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" + if [ -z "$DECKHAND_IMAGE" ]; then log_section "Running Deckhand via uwsgi" - - # Set --workers 2, so that concurrency is always tested. - uwsgi \ - --http :9000 \ - -w deckhand.cmd \ - --callable deckhand_callable \ - --enable-threads \ - --workers 2 \ - --threads 1 \ - -L \ - --pyargv "--config-file $CONF_DIR/deckhand.conf" & + # NOTE(fmontei): Deckhand's database is not configured to work with + # multiprocessing. Currently there is a data race on acquiring shared + # SQLAlchemy engine pooled connection strings when workers > 1. As a + # workaround, we use multiple threads but only 1 worker. For more + # information, see: https://github.com/att-comdev/deckhand/issues/20 + export DECKHAND_API_WORKERS=1 + export DECKHAND_API_THREADS=4 + source $ROOTDIR/../entrypoint.sh & + sleep 5 else log_section "Running Deckhand via Docker" sudo docker run \ @@ -229,8 +229,6 @@ echo $DECKHAND_ID log_section Running tests -ROOTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" - # Create folder for saving HTML test results. if [ ! -d $ROOTDIR/results ]; then mkdir $ROOTDIR/results