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

Performance optimisation. Do not query database for existence of schema upon every call #411

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

NielsKSchjoedt
Copy link

@NielsKSchjoedt NielsKSchjoedt commented Mar 9, 2017

@mikecmpbll
Copy link
Collaborator

thanks! what are the implications of this?

Can you verify what happens if you straight up remove that line and try to connect to an invalid tenant?

@NielsKSchjoedt
Copy link
Author

NielsKSchjoedt commented Mar 10, 2017

It removes the absurd amount of calls to:

  SELECT COUNT(*)
     FROM pg_namespace
     WHERE nspname = ?

No exception is raised though, if you set the path to something like Apartment::Tenant.switch!('sdgjkdfkg') it just does, then you will of cause get an error like ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: relation "users" does not exist, if you try to do User.find(1) afterwards. But I guess that is okay, given that we save all the calls

@caironoleto
Copy link
Contributor

For me, it's ok too but without a specific error can lead to a misunderstood about the problem.

Another way is to check the error to find a pattern like "relation (*) does not exist" and assume that error is because there is no schema. But is a weak solution.

@mikecmpbll
Copy link
Collaborator

mikecmpbll commented Mar 10, 2017

i was looking in to how this relates to the TenantNotFound exception, and oddly it looks as though the pg adapter won't raise TenantNotFound because ActiveRecord::StatementInvalid isn't being caught :/. I assume the intention here was to raise TenantNotFound like the abstract adapter's connect_to_new does.

@jhubert
Copy link

jhubert commented Sep 10, 2020

Fwiw, this is what it does:

image

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.

4 participants