Skip to content
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

Postgres fixes: Concurrency issue, connection leak, and fixes to guard CRUD operations #83

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

AlejandroEsquivel
Copy link
Contributor

@AlejandroEsquivel AlejandroEsquivel commented Nov 1, 2024

Pull Request Type

  • Breaking Change
  • Feature
  • Bug Fix
  • Non-bug Patch (dependency update, non-production code, etc.)

Link to Notion Task or Github Issue

Summary of Feature(s)

Summary of Bug Fix(es)

  • The docker images using gunicorn need to use uvicorn workers (WSGI / ASGI incompatibility) due to the FastAPI switch
  • The containers were crashing upon coming online (workers were dying) due to workers concurrently attempting to execute the sql statements with the audit function, now using advisory lock
  • Using db = next(self.get_db()) was essentially causing connections to proliferate due to not being properly closed, so opted for contextmanager approach
  • Also db = next(self.get_db()) was causing some changes not to commit properly so deletions / updates weren't working properly due to inconsistent states during db.commits

Previous Behaviour

  • Failed to start in Docker due to WSGI / ASGI incompatibility after FastAPI change.
  • In POSTGRES mode workers were dying due to attempting to execute audit function creation SQL statements concurrently.
  • Connection leak due to sessions not being explicitly closed
  • UDATE / DELETE API CRUD operations were not working

New Behaviour

  • No connection leaks
  • UDATE / DELETE API CRUD operations working
  • Multiple workers able to come up reliably while only one worker executes initialization SQL statements with advisory lock

Other details

  • PG Client now has util_ methods, these are utility functions which don't create db connections but rather take db sessions as arguments and perform auxiliary actions for the non util functions which are called directly by API controllers and do start db sessions

Dependencies

@AlejandroEsquivel AlejandroEsquivel marked this pull request as ready for review November 13, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant