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

Optimize user, role and organizations SQL queries #948

Merged
merged 1 commit into from
Oct 17, 2021

Conversation

svenseeberg
Copy link
Member

@svenseeberg svenseeberg commented Sep 24, 2021

This PR aims at creating a checkbox at "users" in #943

Sadly, AFAICT there is not that much performance to gain here.

@svenseeberg svenseeberg force-pushed the bugfix/user-performance branch 6 times, most recently from 4ee83a3 to ffae7be Compare September 25, 2021 10:16
@svenseeberg svenseeberg changed the title Improve user related performance Optimize user, role and organizations SQL queries Sep 25, 2021
@svenseeberg svenseeberg force-pushed the bugfix/user-performance branch 5 times, most recently from ca27c9b to 114affc Compare September 25, 2021 10:58
@svenseeberg svenseeberg mentioned this pull request Sep 25, 2021
16 tasks
@svenseeberg svenseeberg marked this pull request as ready for review September 25, 2021 11:03
@svenseeberg svenseeberg requested a review from a team September 25, 2021 11:04
@svenseeberg svenseeberg force-pushed the bugfix/user-performance branch 2 times, most recently from 256bd7c to 82a8fa8 Compare September 27, 2021 10:37
Copy link
Member

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

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

Thanks! 👍
I just reviewed commits cb23403 and 82a8fa8 which look good to me.

src/cms/views/roles/role_view.py Show resolved Hide resolved
src/cms/views/roles/role_view.py Show resolved Hide resolved
@svenseeberg svenseeberg force-pushed the bugfix/user-performance branch 2 times, most recently from a70d1d5 to f890ad4 Compare September 28, 2021 12:54
Copy link
Contributor

@melegiul melegiul left a comment

Choose a reason for hiding this comment

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

Awesome, select_related is a great choice when it comes to looking up foreign key relations 🚀
Maybe we could use a similar approach for the user groups relationship?
But that could be the next iteration, if at all needed

Copy link
Member

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

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

Prefetching the groups and role objects reduced the amount of queries even further: Instead of doing two additional queries for each user to retrieve the groups and the role of the user, this does now two queries in the beginning to get the groups and roles for all users at once, so a constant number of queries independently from the number of users in the database. 🎉

@svenseeberg
Copy link
Member Author

I'll rebase after the XLIFF PR is merged.

@ulliholtgrave
Copy link
Member

The PR is merged, I guess you can rebase it now :)

- Select user organization along with user data.
- Combine group and role queries into one to improve performance.
- Prefetch group and role objects in user list

Co-authored-by: Timo Ludwig <ludwig@integreat-app.de>
@timobrembeck timobrembeck merged commit c0ec4fa into develop Oct 17, 2021
@timobrembeck timobrembeck deleted the bugfix/user-performance branch October 17, 2021 15:28
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.

None yet

4 participants