From ab497a269a2cc5d1b97cefc2d101947c1af2c820 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 17 Nov 2020 16:01:53 +0100 Subject: [PATCH 1/4] improve messages --- .../api/dependencies/database.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py b/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py index 6e1791d758a..581fabbaaad 100644 --- a/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py +++ b/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py @@ -20,16 +20,18 @@ async def _get_repo( ) -> AsyncGenerator[BaseRepository, None]: logger.debug( - "Acquiring pg connection from pool: current=%d, free=%d, reserved=[%d, %d]", + "Acquiring pg connection from pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", engine.size, + engine.size - engine.freesize, engine.freesize, engine.minsize, engine.maxsize, ) if engine.freesize <= 1: logger.warning( - "Last or no pg connection in pool: current=%d, free=%d, reserved=[%d, %d]", + "Last or no pg connection in pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", engine.size, + engine.size - engine.freesize, engine.freesize, engine.minsize, engine.maxsize, @@ -39,8 +41,9 @@ async def _get_repo( yield repo_type(conn) logger.debug( - "Released pg connection: current=%d, free=%d, reserved=[%d, %d]", + "Released pg connection: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", engine.size, + engine.size - engine.freesize, engine.freesize, engine.minsize, engine.maxsize, From 3b7890d294182ae1165a4ce26716b1c16986c2bd Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 17 Nov 2020 17:23:35 +0100 Subject: [PATCH 2/4] a fastapi depency is cached and thus the same connection is given per request --- .../api/dependencies/database.py | 43 +++++++++++-- .../api/dependencies/database.py | 61 ++++++++++-------- .../api/dependencies/database.py | 62 ++++++++++--------- 3 files changed, 106 insertions(+), 60 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/api/dependencies/database.py b/services/api-server/src/simcore_service_api_server/api/dependencies/database.py index ef94b71e26c..9c5f3728ba5 100644 --- a/services/api-server/src/simcore_service_api_server/api/dependencies/database.py +++ b/services/api-server/src/simcore_service_api_server/api/dependencies/database.py @@ -1,21 +1,56 @@ +import logging from typing import AsyncGenerator, Callable, Type -from aiopg.sa import Engine +from aiopg.sa import Engine, SAConnection from fastapi import Depends from fastapi.requests import Request from ...db.repositories import BaseRepository +logger = logging.getLogger(__name__) + def _get_db_engine(request: Request) -> Engine: return request.app.state.engine +async def _acquire_connection(engine: Engine = Depends(_get_db_engine)) -> SAConnection: + logger.debug( + "Acquiring pg connection from pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", + engine.size, + engine.size - engine.freesize, + engine.freesize, + engine.minsize, + engine.maxsize, + ) + if engine.freesize <= 1: + logger.warning( + "Last or no pg connection in pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", + engine.size, + engine.size - engine.freesize, + engine.freesize, + engine.minsize, + engine.maxsize, + ) + + async with engine.acquire() as conn: + yield conn + + logger.debug( + "Released pg connection: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", + engine.size, + engine.size - engine.freesize, + engine.freesize, + engine.minsize, + engine.maxsize, + ) + + def get_repository(repo_type: Type[BaseRepository]) -> Callable: async def _get_repo( - engine: Engine = Depends(_get_db_engine), + db_connection: SAConnection = Depends(_acquire_connection), ) -> AsyncGenerator[BaseRepository, None]: - async with engine.acquire() as conn: - yield repo_type(conn) + + yield repo_type(db_connection) return _get_repo diff --git a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py index 3fce97d2510..9c5f3728ba5 100644 --- a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py +++ b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py @@ -1,7 +1,7 @@ import logging from typing import AsyncGenerator, Callable, Type -from aiopg.sa import Engine +from aiopg.sa import Engine, SAConnection from fastapi import Depends from fastapi.requests import Request @@ -14,36 +14,43 @@ def _get_db_engine(request: Request) -> Engine: return request.app.state.engine -def get_repository(repo_type: Type[BaseRepository]) -> Callable: - async def _get_repo( - engine: Engine = Depends(_get_db_engine), - ) -> AsyncGenerator[BaseRepository, None]: - - logger.debug( - "Acquiring pg connection from pool: current=%d, free=%d, reserved=[%d, %d]", - engine.size, - engine.freesize, - engine.minsize, - engine.maxsize, - ) - if engine.freesize <= 1: - logger.warning( - "Last or no pg connection in pool: current=%d, free=%d, reserved=[%d, %d]", - engine.size, - engine.freesize, - engine.minsize, - engine.maxsize, - ) - - async with engine.acquire() as conn: - yield repo_type(conn) - - logger.debug( - "Released pg connection: current=%d, free=%d, reserved=[%d, %d]", +async def _acquire_connection(engine: Engine = Depends(_get_db_engine)) -> SAConnection: + logger.debug( + "Acquiring pg connection from pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", + engine.size, + engine.size - engine.freesize, + engine.freesize, + engine.minsize, + engine.maxsize, + ) + if engine.freesize <= 1: + logger.warning( + "Last or no pg connection in pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", engine.size, + engine.size - engine.freesize, engine.freesize, engine.minsize, engine.maxsize, ) + async with engine.acquire() as conn: + yield conn + + logger.debug( + "Released pg connection: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", + engine.size, + engine.size - engine.freesize, + engine.freesize, + engine.minsize, + engine.maxsize, + ) + + +def get_repository(repo_type: Type[BaseRepository]) -> Callable: + async def _get_repo( + db_connection: SAConnection = Depends(_acquire_connection), + ) -> AsyncGenerator[BaseRepository, None]: + + yield repo_type(db_connection) + return _get_repo diff --git a/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py b/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py index 581fabbaaad..953dc468e54 100644 --- a/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py +++ b/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py @@ -1,7 +1,7 @@ import logging from typing import AsyncGenerator, Callable, Type -from aiopg.sa import Engine +from aiopg.sa import Engine, SAConnection from fastapi import Depends from fastapi.requests import Request @@ -14,34 +14,18 @@ def _get_db_engine(request: Request) -> Engine: return request.app.state.engine -def get_repository(repo_type: Type[BaseRepository]) -> Callable: - async def _get_repo( - engine: Engine = Depends(_get_db_engine), - ) -> AsyncGenerator[BaseRepository, None]: - - logger.debug( - "Acquiring pg connection from pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", - engine.size, - engine.size - engine.freesize, - engine.freesize, - engine.minsize, - engine.maxsize, - ) - if engine.freesize <= 1: - logger.warning( - "Last or no pg connection in pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", - engine.size, - engine.size - engine.freesize, - engine.freesize, - engine.minsize, - engine.maxsize, - ) - - async with engine.acquire() as conn: - yield repo_type(conn) - - logger.debug( - "Released pg connection: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", +async def _acquire_connection(engine: Engine = Depends(_get_db_engine)) -> SAConnection: + logger.debug( + "Acquiring pg connection from pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", + engine.size, + engine.size - engine.freesize, + engine.freesize, + engine.minsize, + engine.maxsize, + ) + if engine.freesize <= 1: + logger.warning( + "Last or no pg connection in pool: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", engine.size, engine.size - engine.freesize, engine.freesize, @@ -49,4 +33,24 @@ async def _get_repo( engine.maxsize, ) + async with engine.acquire() as conn: + yield conn + + logger.debug( + "Released pg connection: pool size=%d, acquired=%d, free=%d, reserved=[%d, %d]", + engine.size, + engine.size - engine.freesize, + engine.freesize, + engine.minsize, + engine.maxsize, + ) + + +def get_repository(repo_type: Type[BaseRepository]) -> Callable: + async def _get_repo( + db_connection: SAConnection = Depends(_acquire_connection), + ) -> AsyncGenerator[BaseRepository, None]: + + yield repo_type(db_connection) + return _get_repo From f502ca1bafb4df7a6d780ecadcf9c747011d8f80 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 17 Nov 2020 17:54:10 +0100 Subject: [PATCH 3/4] added fix to convert FAILURE to FAILED --- ...d1c43b5d33_migrate_workbench_state_enum.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 packages/postgres-database/src/simcore_postgres_database/migration/versions/cfd1c43b5d33_migrate_workbench_state_enum.py diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/cfd1c43b5d33_migrate_workbench_state_enum.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/cfd1c43b5d33_migrate_workbench_state_enum.py new file mode 100644 index 00000000000..7c24e0e7bd8 --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/cfd1c43b5d33_migrate_workbench_state_enum.py @@ -0,0 +1,34 @@ +"""migrate workbench state enum + +Revision ID: cfd1c43b5d33 +Revises: c8a7073deebb +Create Date: 2020-11-17 16:42:32.511722+00:00 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'cfd1c43b5d33' +down_revision = 'c8a7073deebb' +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute( + sa.DDL( + """ +UPDATE projects + SET workbench = (regexp_replace(workbench::text, '"FAILURE"', '"FAILED"'))::json + WHERE workbench::text LIKE '%%FAILURE%%' + """ + ) + ) + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + pass + # ### end Alembic commands ### From b48c55a7de1d74b7c751dd9ff5c33ab0688bc451 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 17 Nov 2020 18:00:59 +0100 Subject: [PATCH 4/4] added comment --- .../simcore_service_api_server/api/dependencies/database.py | 3 ++- .../src/simcore_service_catalog/api/dependencies/database.py | 3 ++- .../simcore_service_director_v2/api/dependencies/database.py | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/api/dependencies/database.py b/services/api-server/src/simcore_service_api_server/api/dependencies/database.py index 9c5f3728ba5..1637b845a44 100644 --- a/services/api-server/src/simcore_service_api_server/api/dependencies/database.py +++ b/services/api-server/src/simcore_service_api_server/api/dependencies/database.py @@ -50,7 +50,8 @@ def get_repository(repo_type: Type[BaseRepository]) -> Callable: async def _get_repo( db_connection: SAConnection = Depends(_acquire_connection), ) -> AsyncGenerator[BaseRepository, None]: - + # NOTE: Since _acquire_connection is a dependency, it is a cached by FastApi and is only called once per request + # Be very careful if you change this!!! or we will end up in the problem described in https://github.com/ITISFoundation/osparc-simcore/pull/1966 yield repo_type(db_connection) return _get_repo diff --git a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py index 9c5f3728ba5..1637b845a44 100644 --- a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py +++ b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py @@ -50,7 +50,8 @@ def get_repository(repo_type: Type[BaseRepository]) -> Callable: async def _get_repo( db_connection: SAConnection = Depends(_acquire_connection), ) -> AsyncGenerator[BaseRepository, None]: - + # NOTE: Since _acquire_connection is a dependency, it is a cached by FastApi and is only called once per request + # Be very careful if you change this!!! or we will end up in the problem described in https://github.com/ITISFoundation/osparc-simcore/pull/1966 yield repo_type(db_connection) return _get_repo diff --git a/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py b/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py index 953dc468e54..1e195ace6f7 100644 --- a/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py +++ b/services/director-v2/src/simcore_service_director_v2/api/dependencies/database.py @@ -50,7 +50,8 @@ def get_repository(repo_type: Type[BaseRepository]) -> Callable: async def _get_repo( db_connection: SAConnection = Depends(_acquire_connection), ) -> AsyncGenerator[BaseRepository, None]: - + # NOTE: Since _acquire_connection is a dependency, it is a cached by FastApi and is only called once per request + # Be very careful if you change this!!! or we will end up in the problem described in https://github.com/ITISFoundation/osparc-simcore/pull/1966 yield repo_type(db_connection) return _get_repo