Refactor database connection handling to reduce memory usage and improve efficiency#101
Refactor database connection handling to reduce memory usage and improve efficiency#101FelipePegado wants to merge 10 commits into
Conversation
Use contextlib.closing to ensure psycopg2 connections are closed immediately after use instead of relying on GC. Add try/finally to async_db_fetch and async_db_transaction so asyncpg pools are always closed even when exceptions occur. Add explicit con.commit() for write operations to preserve transactional semantics.
|
Hey mate, thanks for taking the time to submit this pr. I'll have a look at this, using the context manager is the correct way to ensure and roll back transactions if something goes wrong. force closing the connection and clean up on the pool will cause its own issues. i think the correct course of action would be to create a database pool every connection acquires a connection to and releases from as well as having all code async. In this instance it does get a bit messy as I have mixed static and async code around and the connectors for each which is not ideal. a quick run down if you're interested.
|
|
Thanks for the feedback. Yeah, I agree that the right long-term fix is a single shared pool created at startup with fully async DB access. But since that would be a larger architectural change that we hadn’t discussed yet, I kept this PR focused on fixing the leak. I’ll work on the broader refactor this weekend, then. |
Database now takes a DSN and creates the pool once (connect) / closes it once (close), instead of being instantiated per call.
db_transaction/db_fetch/fetch_active_devices/create_tables are now async and acquire connections from the shared pool. __init__ takes a pool instead of the postgres_* params.
db_transaction/db_fetch/stream_meta/meta_up are now async over the shared pool and redis.asyncio. Sync usage publishers run via run_in_executor. stream_meta's try/except now wraps the loop body so a transient error no longer kills the stream permanently.
Replaces the ThreadPoolExecutor (one event loop pe0r thread) with a single asyncio.run(main()) that creates one shared pool and runs all five tasks via asyncio.gather. Periodic tasks use an async run_periodically helper instead of asyncio.run per tick. build_dsn now applies POSTGRES_SSL_MODE correctly (the old check compared a single char and never matched).
create_pool defaults to min_size=10/max_size=10, so the service held 10 Postgres connections open for its whole lifetime. On a database shared with ChirpStack that can push it past max_connections. Cap the pool at max_size=5 and min_size=1 so idle connections are released.
2 participants
Fix #100
I will let this application run for a while and monitor the memory usage to see if it remains stable.