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

Potential Deadlock due to DB Connection Pool Contention in GraphQL #3286

Open
mastercactapus opened this issue Sep 18, 2023 · 1 comment
Open
Labels
bug Something isn't working

Comments

@mastercactapus
Copy link
Member

Describe the Bug:
When concurrently operating with GraphQL in GoAlert, the DB connection pool can experience contention. One scenario is when 15 concurrent resolvers initiate transactions, and a side dependency tries to fetch data outside these transactions, causing a deadlock. This is a bug because it can negatively impact the functionality and performance of the system.

Steps to Reproduce:
This deadlock could be reproduced by forcing contention on the DB connection pool:

  1. Set max connections to < 3
  2. Run concurrent GraphQL queries and watch the logs

Note: this is an extreme case, but is an easy way to demonstrate the issue

Expected Behavior:
Multiple concurrent operations should be handled without causing a deadlock or exhausting the DB connection pool.

Observed Behavior:
High concurrency in GraphQL operations can cause DB connection pool saturation, leading to a deadlock.

Additional Context:
We think potential areas where this issue could arise should be looked over closely, and the code should be updated to operate in an existing transaction for such scenarios. Consideration should be given to a dedicated connection pool for behind-the-scenes operations (like engine, key management, and configuration) from API requests to isolate those concerns. A possible solution could also involve the sub-pooling mechanism to limit the number of connections for individual requests.

Many store methods accept a sql.Tx but those that do not will require a second connection to be used if the request is already operating in a transaction.

@mastercactapus
Copy link
Member Author

Note: One potential option could be to limit the withContextTx to only create a single transaction per request, at a time. Alternatively, we could look into limiting DB access to 2 concurrent calls per request, at a time (allowing for a tx + cache update to not deadlock).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant