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

Add multi-user licenses (again) #802

Merged
merged 197 commits into from
Apr 25, 2024
Merged

Conversation

ezekg
Copy link
Member

@ezekg ezekg commented Mar 1, 2024

Closes #534. Reverted #774 due to performance issues with union_of.

The issues are mainly i.r.t. INNER JOIN ... ON id IN (SELECT id FROM ...) joins. For large accounts, performance is just not where I want it. E.g. an account with 600k licenses takes 2s to load a user's machines, because we have to join across 600k licenses to see if the user has any associated licenses with other users, so that we can list the user's teammates' machines as well as the user's machines. The same issue is present when listing a user's licenses, and a user's teammates.

I tried optimizing a few queries by hand, but there's just too many joins on the users union association. We're going to need to optimize union_of itself so that we don't do an JOIN ... ON id IN (SELECT id FROM ...), since that could potentially select hundreds of thousands or even millions of IDs, which is not memory efficient (not to mention slow).

This is what I get for skipping performance testing.

Prerequisites

Pre-deploy

  • Create license.users.attached, license.users.detached, license.owner.updated, machine.owner.updated event types.
  • Create license.users.attach, license.users.detach, license.owner.update, machine.owner.update permissions.
  • Optimize autovacuum settings (especially the users table).
  • Manually vacuum+analyze users and licenses tables.
  • Enable query logging for denormalization migration.

Post-deploy

  • Run VACUUM ANALYZE licenses.
  • Run VACUUM ANALYZE users.
  • Run the below Rake tasks.
# Add new permissions and event types
rake db:seed

# Migrate existing role permissions
rake keygen:permissions:admins:add[license.users.attach,license.users.detach,license.owner.update,machine.owner.update]
rake keygen:permissions:environments:add[license.users.attach,license.users.detach,license.owner.update,machine.owner.update]
rake keygen:permissions:products:add[license.users.attach,license.users.detach,license.owner.update,machine.owner.update]

@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch 3 times, most recently from 0c02fda to bdbf11d Compare March 9, 2024 08:40
spec/lib/union_of_spec.rb Outdated Show resolved Hide resolved
@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch 3 times, most recently from b5bedcc to 9c440e4 Compare March 20, 2024 01:25
@ezekg
Copy link
Member Author

ezekg commented Mar 23, 2024

Here are some queries that seem to perform well, and are able to be indexed efficiently:

-- licenses for user
select licenses.* from licenses left outer join license_users on license_users.license_id = licenses.id and license_users.user_id = '598840ca-c529-40fd-9d9b-fe650619726a' and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and (licenses.user_id = '598840ca-c529-40fd-9d9b-fe650619726a' or license_users.user_id = '598840ca-c529-40fd-9d9b-fe650619726a') order by licenses.created_at desc limit 10;

-- users for product
select distinct users.* from users left outer join license_users on license_users.user_id = users.id and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join licenses on (licenses.user_id = users.id or licenses.id = license_users.license_id) and licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join policies on policies.id = licenses.policy_id and policies.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and policies.product_id = '00dda7a7-deb7-4a76-8a84-2be19adde374' order by users.created_at desc limit 10;

-- users for product (denormalized)
select distinct users.* from users left outer join license_users on license_users.user_id = users.id and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join licenses on (licenses.user_id = users.id or licenses.id = license_users.license_id) and licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and licenses.product_id = '00dda7a7-deb7-4a76-8a84-2be19adde374' order by users.created_at desc limit 10;

-- users for license
select distinct users.* from users left outer join license_users on license_users.user_id = users.id and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join licenses on (licenses.user_id = users.id or licenses.id = license_users.license_id) and licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and licenses.id = 'aa8ac994-a3ed-4c06-89fd-4c11736b6c55' order by users.created_at desc limit 10;

-- users for machine
select distinct users.* from users left outer join license_users on license_users.user_id = users.id and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join licenses on (licenses.user_id = users.id or licenses.id = license_users.license_id) and licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join machines on machines.license_id = licenses.id and machines.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and machines.id = 'db10ee10-a1ac-41c4-a7c9-332a534fe398' order by users.created_at desc limit 10;

@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch 9 times, most recently from cf8ebce to f024ece Compare March 26, 2024 21:41
end
}

describe '.denormalizes' do
Copy link
Member Author

@ezekg ezekg Mar 26, 2024

Choose a reason for hiding this comment

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

Add more tests. Would be really nice to introduce temp tables for this and union_of.

@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch 3 times, most recently from 271eaa0 to af3e1a2 Compare March 27, 2024 21:22
@ezekg ezekg marked this pull request as ready for review March 27, 2024 21:24
@ezekg ezekg marked this pull request as draft March 27, 2024 21:25
spec/models/policy_spec.rb Outdated Show resolved Hide resolved
@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch from 6f148f6 to db611b3 Compare April 2, 2024 17:30
app/models/user.rb Show resolved Hide resolved
app/models/user.rb Outdated Show resolved Hide resolved
@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch from 849c33f to 40da878 Compare April 25, 2024 01:55
@ezekg ezekg merged commit 3e02d87 into master Apr 25, 2024
5 checks passed
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.

A license should have multiple users
1 participant