Skip to content

Optimize group operations #45

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

joshuata
Copy link
Contributor

This makes a few changes to the way groups are handled in order to reduce the number of queries executed for each login.

  1. Groups are created once at startup. Previously this executed one query per line in the role mapping on every login. This was redundant and scales poorly with large mappings or lots of users.
  2. Previously the user's groups were queried one time for each entry in the role mapping to determine whether a group should be removed. Now the groups are queried a single time to return all relations.
  3. Adding and removing groups was done individually, now it is done in batches, and only if there are items to add or remove.

The result of these changes is that authenticating a user now executes strictly fewer queries and a constant, rather than linear, number.

If the mapping has n entries, here is the change in query count for different situations:

Situation Previous New
No group changes needed 2n 1
Add x groups 2n 2
Remove y groups 2n + y 2
Add x, remove y 2n + y 3

The add and remove queries could be combined, however a user may be a member of many groups, and this would require using the explicit set method and sending the name of every group in the query. This would both increase bandwidth as well as potentially leading to database portability issues.

@joshuata
Copy link
Contributor Author

Let's hold off on this one for a few days so I can add some more tests. I want to make sure I didn't change any logic, and I can add regression tests to make sure it maintains a constant number of queries regardless of the number of groups

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.

1 participant