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

Speed up introspection of system objects like Database and Role #6665

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

msullivan
Copy link
Member

@msullivan msullivan commented Jan 4, 2024

Currently if there are a lot of roles, doing a query like select sys::Role { member_of } is quite slow. This is because the views for
both Role and member_of need to extract metadata for every Role
and pase it as json, and postgres is planning this as a nested loop.

Unfortunately, startup needs to query all these objects, so having
lots of databases or roles is a problem.

Fix this by forcing sys objects and links to always be materialized
into CTEs. This ensures that we do at most one linear scan of each of
the views. Since there is no way to hit an index when querying any of
these system object views, always doing a linear scan is no worse.

For the objects themselves we can leverage the rewrites mechanism. For
link tables, we add a slightly lighter weight mechanism to handle it.

Tested using 100 roles.

Reverts the now-unnecessary main part of #6633 but keeps the extension
to the patch system. We could actually put annotations on databases now
and have it not be too slow.

This can be backported with an empty edgeql+schema+globalonly patch.

Currently if there are a lot of roles, doing a query like `select
sys::Role { member_of }` is quite slow. This is because the views for
both `Role` and `member_of` need to extract metadata for *every* Role
and pase it as json, and postgres is planning this as a nested loop.

Unfortunately, startup needs to query all these objects, so having
lots of databases or roles is a problem.

Fix this by forcing sys objects and links to always be materialized
into CTEs. This ensures that we do at most one linear scan of each of
the views. Since there is no way to hit an index when querying any of
these system object views, always doing a linear scan is no worse.

For the objects themselves we can leverage the rewrites mechanism. For
link tables, we add a slightly lighter weight mechanism to handle it.

Reverts the now-unnecessary main part of #6633 but keeps the extension
to the patch system. We could actually put annotations on databases now
and have it not be *too* slow.

This can be backported with an empty `edgeql+schema+globalonly` patch.
Copy link
Contributor

@aljazerzen aljazerzen left a comment

Choose a reason for hiding this comment

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

This is a clean solution, I like it.

@msullivan msullivan merged commit 096f791 into master Jan 17, 2024
21 checks passed
@msullivan msullivan deleted the role-opt2 branch January 17, 2024 01:15
aljazerzen pushed a commit that referenced this pull request Jan 25, 2024
Currently if there are a lot of roles, doing a query like `select
sys::Role { member_of }` is quite slow. This is because the views for
both `Role` and `member_of` need to extract metadata for *every* Role
and pase it as json, and postgres is planning this as a nested loop.

Unfortunately, startup needs to query all these objects, so having
lots of databases or roles is a problem.

Fix this by forcing sys objects and links to always be materialized
into CTEs. This ensures that we do at most one linear scan of each of
the views. Since there is no way to hit an index when querying any of
these system object views, always doing a linear scan is no worse.

For the objects themselves we can leverage the rewrites mechanism. For
link tables, we add a slightly lighter weight mechanism to handle it.

Reverts the now-unnecessary main part of #6633 but keeps the extension
to the patch system. We could actually put annotations on databases now
and have it not be *too* slow.

This can be backported with an empty `edgeql+schema+globalonly` patch.
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.

2 participants