Reduce Idle Connections from Health Checks [#1102] (#1107)

* Don't create a new engine as part of running the health checks and share a single engine across the application, including for the health checks.  Currently we're using the default pool_size and max_overflow.

* Update changelog.

* Fix that health checks are still supposed to run, even if the database is disabled.

* Need to yield instead -  'generator' object has no attribute 'query'

CHANGELOG.md

@@ -34,6 +34,7 @@ The types of changes are:
 
 * HTTP headers are now preserved in requests generated from SaaS connector pagination [#1069](https://github.com/ethyca/fidesops/pull/1069)
 * Bump fideslib to fix issue where the authenticate button in the FastAPI docs did not work [#1092](https://github.com/ethyca/fidesops/pull/1092)
+* Reduced number of connections opened against app db during health checks [#1107](https://github.com/ethyca/fidesops/pull/1107)
 
 ### Changed
 

src/fidesops/ops/api/deps.py

@@ -1,26 +1,48 @@
 from typing import Generator
 
-from fideslib.db.session import get_db_session
+from fideslib.db.session import get_db_engine, get_db_session
+from sqlalchemy.orm import Session
 
 from fidesops.ops.common_exceptions import FunctionalityNotConfigured
 from fidesops.ops.core.config import config
 from fidesops.ops.util.cache import get_cache as get_redis_connection
 
+_engine = None
+
 
 def get_db() -> Generator:
     """Return our database session"""
     if not config.database.enabled:
         raise FunctionalityNotConfigured(
             "Application database required, but it is currently disabled! Please update your application configuration to enable integration with an application database."
         )
+
+    try:
+        db = _get_session()
+        yield db
+    finally:
+        db.close()
+
+
+def get_db_for_health_check() -> Generator:
+    """Gets a database session regardless of whether the application db is disabled, for a health check."""
     try:
-        SessionLocal = get_db_session(config)
-        db = SessionLocal()
+        db = _get_session()
         yield db
     finally:
         db.close()
 
 
+def _get_session() -> Session:
+    """Gets a database session"""
+    global _engine  # pylint: disable=W0603
+    if not _engine:
+        _engine = get_db_engine(config=config)
+    SessionLocal = get_db_session(config, engine=_engine)
+    db = SessionLocal()
+    return db
+
+
 def get_cache() -> Generator:
     """Return a connection to our redis cache"""
     if not config.redis.enabled:

src/fidesops/ops/api/v1/endpoints/health_endpoints.py

@@ -2,9 +2,11 @@
 from typing import Dict, Optional, Union
 
 from alembic import migration, script
+from fastapi import Depends
 from redis.exceptions import ResponseError
-from sqlalchemy import create_engine
+from sqlalchemy.orm import Session
 
+from fidesops.ops.api import deps
 from fidesops.ops.api.v1.urn_registry import HEALTH
 from fidesops.ops.common_exceptions import RedisConnectionError
 from fidesops.ops.core.config import config
@@ -21,29 +23,29 @@
 
 
 @router.get(HEALTH, response_model=Dict[str, Union[bool, str]])
-def health_check() -> Dict[str, Union[bool, str]]:
+def health_check(
+    db: Session = Depends(deps.get_db_for_health_check),
+) -> Dict[str, Union[bool, str]]:
     return {
         "webserver": "healthy",
-        "database": get_db_health(config.database.sqlalchemy_database_uri),
+        "database": get_db_health(config.database.sqlalchemy_database_uri, db),
         "cache": get_cache_health(),
     }
 
 
-def get_db_health(database_url: Optional[str]) -> str:
+def get_db_health(database_url: Optional[str], db: Session) -> str:
     """Checks if the db is reachable and up to date in alembic migrations"""
     if not database_url or not config.database.enabled:
         return "no db configured"
     try:
-        engine = create_engine(database_url)
         alembic_config = get_alembic_config(database_url)
         alembic_script_directory = script.ScriptDirectory.from_config(alembic_config)
-        with engine.begin() as conn:
-            context = migration.MigrationContext.configure(conn)
-            if (
-                context.get_current_revision()
-                != alembic_script_directory.get_current_head()
-            ):
-                return "needs migration"
+        context = migration.MigrationContext.configure(db.connection())
+        if (
+            context.get_current_revision()
+            != alembic_script_directory.get_current_head()
+        ):
+            return "needs migration"
         return "healthy"
     except Exception as error:  # pylint: disable=broad-except
         logger.error(f"Unable to reach the database: {error}")