- 
                Notifications
    
You must be signed in to change notification settings  - Fork 32
 
🎨Maintenance & Performance: Properly setup asyncpg DB engine #8322
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
🎨Maintenance & Performance: Properly setup asyncpg DB engine #8322
Conversation
          Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@            Coverage Diff             @@
##           master    #8322      +/-   ##
==========================================
- Coverage   87.85%   87.84%   -0.02%     
==========================================
  Files        1945     1945              
  Lines       75370    75371       +1     
  Branches     1311     1311              
==========================================
- Hits        66218    66208      -10     
- Misses       8757     8768      +11     
  Partials      395      395              
 
 Continue to review full report in Codecov by Sentry. 
 🚀 New features to boost your workflow:
  | 
    
          
🧪 CI InsightsHere's what we observed from your CI run for 1cad715. ✅ Passed Jobs With Interesting Signals
  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx! Really appreciated!
7a421f7    to
    865ca16      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reconfigures the PostgreSQL database connection pool settings for the transition from aiopg to asyncpg. The main purpose is to address performance issues that arose after the migration by properly setting up the database engine with appropriate connection pool parameters.
Key changes:
- Add new environment variables 
POSTGRES_MAX_POOLSIZEandPOSTGRES_MAX_OVERFLOWfor asyncpg-specific configuration - Redefine default values for 
POSTGRES_MINSIZE(from 2 to 1) and maintainPOSTGRES_MAXSIZEat 50 - Consolidate PostgreSQL environment variables in docker-compose.yml using YAML anchors for better maintainability
 
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description | 
|---|---|
| services/docker-compose.yml | Introduces x-postgres-settings YAML anchor and applies it across all services, removing duplicate environment variable definitions | 
| packages/settings-library/src/settings_library/postgres.py | Adds POSTGRES_MAX_POOLSIZE and POSTGRES_MAX_OVERFLOW fields, updates field descriptions and default values | 
| packages/service-library/src/servicelib/db_asyncpg_utils.py | Updates engine creation to use new pool settings and adds orjson serialization | 
| services/web/server/src/simcore_service_webserver/application_settings_utils.py | Maps new PostgreSQL settings to application configuration | 
| Multiple test files | Updates test configurations with new environment variables and default values | 
1550aa5    to
    86176b6      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good stuff, lgtm thx
          
 | 
    



What do these changes do?
The interface for the creation of the SqlAlchemy DB connection using asyncpg is slightly different from aiopg. Creating the engine have different arguments that have quite an influence on the connections pool:
asyncpg
pool_size--> this is the maximum amount of connections that are kept alive (aka hot)max_overflow--> this is the maximum amount of connections overpool_sizethat can be created on the fly on demand and that are discarded right away (aka slow)aiopg
min_size--> on start creates this amount of connectionsmax_size--> pool might go up to that amount of connectionsWe are migrating to asyngp currently.
Since we are setting the ENV
POSTGRES_MINSIZEtopool_sizethat means that the number of fast connection is currently set to 2, which is very low. That might explain why the performance went down.This PR:
POSTGRES_MAX_POOLSIZEthat is defaulted to 10 and represents the sqlalchemy pool max size (without overflow) - e.g. these connections are kept after initial connection to the postgres DBPOSTGRES_MAX_OVERFLOWthat is defaulted to 20 and represents the sqlalchemy max overflow size - e.g. these are connection created abovePOSTGRES_MAX_POOLSIZEand that are discarded after use (slow construction,deletion)docker-compose.ymlvia yaml anchorsPOSTGRES_MINSIZEof 1 andPOSTGRES_MAXSIZEof 50 as basis instead of 1 and 50 (these are now only used with aiopg engine and the webserver login asyncpg-based manual connection).POSTGRES_ENDPOINTthat is not necessaryRelated issue/s
How to test
Dev-ops