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

fix: Disconnect when tenant has no users #701

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

filipecabaco
Copy link
Contributor

What kind of change does this PR introduce?

To ensure that we limit the amount of connections to a tenant database, we will check if that tenant has any users and if not, we kill the database connection.

@filipecabaco filipecabaco requested review from chasers, abc3 and a team October 17, 2023 22:51
@vercel
Copy link

vercel bot commented Oct 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
realtime-demo ⬜️ Ignored (Inspect) Visit Preview Oct 20, 2023 4:08pm

1 similar comment
@filipecabaco filipecabaco force-pushed the fix/disconnect-from-tenant-on-no-users branch from 20af583 to 56393c4 Compare October 18, 2023 00:08
@filipecabaco filipecabaco force-pushed the fix/disconnect-from-tenant-on-no-users branch from c008137 to c0e5045 Compare October 20, 2023 00:42

def handle_info(:shutdown, %{db_conn_pid: db_conn_pid} = state) do
Logger.info("Tenant has no connected users, database connection will be terminated")
:ok = GenServer.stop(db_conn_pid)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens after this? Is the process going to keep checking and trying to kill a db_conn_pid that doesn't exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're monitoring the reference of this connection it will be caught by the handle info for that effect and kill itself

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roger. Okay LGTM.

@chasers
Copy link
Contributor

chasers commented Oct 20, 2023

@filipecabaco take the nits if you want and feel free to merge whenever.

@filipecabaco filipecabaco force-pushed the fix/disconnect-from-tenant-on-no-users branch from 776dcd3 to cff7818 Compare October 20, 2023 15:46
To ensure that we limit the amount of connections to a tenant database, we will check if that tenant has any users and if not, we kill the database connection.
@filipecabaco filipecabaco force-pushed the fix/disconnect-from-tenant-on-no-users branch from cff7818 to 6991704 Compare October 20, 2023 16:08
@filipecabaco filipecabaco merged commit e14c1c8 into main Oct 20, 2023
3 checks passed
@filipecabaco filipecabaco deleted the fix/disconnect-from-tenant-on-no-users branch October 20, 2023 16:23
@kiwicopple
Copy link
Member

🎉 This PR is included in version 2.25.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants