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

Provide an internal function to deactivate multiple users more efficiently #19490

Open
alexmv opened this issue Aug 3, 2021 · 12 comments · May be fixed by #29668
Open

Provide an internal function to deactivate multiple users more efficiently #19490

alexmv opened this issue Aug 3, 2021 · 12 comments · May be fixed by #29668
Labels
area: performance Performance and scalability issues area: production

Comments

@alexmv
Copy link
Collaborator

alexmv commented Aug 3, 2021

The do_deactivate_user function does what it needs to. However, the work it does is very inefficient if it's deleting a large number of users at once, as delete_user_sessions walks all sessions in the database once for each user that is deactivated.

We should consider adding a function which is designed for bulk deactivation and only requires walking the Sessions table once.

On the flipside, deactivating a large number of users is not a common operation.

@timabbott timabbott added area: performance Performance and scalability issues area: production labels Aug 5, 2021
@zulipbot
Copy link
Member

zulipbot commented Aug 5, 2021

Hello @zulip/server-production members, this issue was labeled with the "area: production" label, so you may want to check it out!

@timabbott
Copy link
Sponsor Member

Probably the right way to do this is to rename it to do_deactivate_users with a loop and have existing callers pass a single user ID, and then start optimizing to use bulk queries, using profiling / query logging to figure out what we need.

@iamakkkhil
Copy link

@timabbott can I have a detailed explanation on this, I am newcomer to Zulip but already have some experience with Django and I would I like to proceed with this issue.

@timabbott
Copy link
Sponsor Member

@iamakkkhil sorry for the slow reply -- I was on vacation at the time. I think this is a pure refactoring function; first, refactor delete_user_sessions to take a list of UserProfile objects and delete sessions for every one in the set. And then do the same with do_deactivate_users, calling delete_user_sessions once rather than in a loop.

(Use git grep to find all the callers and migrate them)

@timabbott
Copy link
Sponsor Member

timabbott commented Apr 15, 2022

Just adding a note that I think the path forward we're likely to take for the core issue here involves extending the Session table to record the User ID and Realm with database indexes, which is different from the previous proposal.

@abdelrahman725
Copy link
Contributor

@zulipbot claim

@zulipbot
Copy link
Member

Hello @abdelrahman725!

Thanks for your interest in Zulip! You have attempted to claim an issue without the label "help wanted". You can only claim and submit pull requests for issues with the help wanted label.

If this is your first time here, we recommend reading our guide for new contributors before getting started.

@zulipbot
Copy link
Member

zulipbot commented Mar 24, 2024

@abdelrahman725 You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!

@abdelrahman725
Copy link
Contributor

@zulipbot still working on it

@zulipbot
Copy link
Member

ERROR: You have already claimed this issue.

@abdelrahman725
Copy link
Contributor

@zulipbot claim

@zulipbot
Copy link
Member

zulipbot commented Apr 9, 2024

Hello @abdelrahman725!

Thanks for your interest in Zulip! You have attempted to claim an issue without the label "help wanted". You can only claim and submit pull requests for issues with the help wanted label.

If this is your first time here, we recommend reading our guide for new contributors before getting started.

abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 10, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with fields: realm, user and ip_address,
indexed by realm and user.

Migrate code to use the new model, zerver/lib/sessions' has most impact.

This change introduced one extra query for 'assert_database_query_count()'
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 17, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with fields: realm, user and ip_address,
indexed by realm and user.

Migrate code to use the new model, zerver/lib/sessions' has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 17, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with fields: realm, user and ip_address,
indexed by realm and user.

Migrate code to use the new model, zerver/lib/sessions' has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 17, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with fields: realm, user and ip_address,
indexed by realm and user.

Migrate code to use the new model, zerver/lib/sessions' has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 17, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with fields: realm, user and ip_address,
indexed by realm and user.

Migrate code to use the new model, zerver/lib/sessions' has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 17, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with fields: realm, user and ip_address,
indexed by realm and user.

Migrate code to use the new model, zerver/lib/sessions' has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 17, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with fields: realm, user and ip_address,
with combined index on (realm, user) and an index on ip_address.

Migrate code to use the new model, zerver/lib/sessions' has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 18, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use the new model, zerver/lib/sessions' has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 21, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use the new model, zerver/lib/sessions' has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 29, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that new model, "zerver/lib/sessions" has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 29, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that new model, "zerver/lib/sessions" has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Apr 29, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that model, "zerver/lib/sessions" has most impact.

Create data_migration to copy over data from
django_session to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue May 9, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that model, "zerver/lib/sessions" has most impact.

Create data_migration to copy over data from
django_session table to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue May 9, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that model, "zerver/lib/sessions" has most impact.

Create data_migration to copy over data from
django_session table to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue May 14, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that model, "zerver/lib/sessions" has most impact.

Create data_migration to copy over data from
django_session table to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue May 14, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that model, "zerver/lib/sessions" has most impact.

Create data_migration to copy over data from
django_session table to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue May 14, 2024
Fixes zulip#19490 and has other performance benefits.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that model, "zerver/lib/sessions" has most impact.

Create data_migration to copy over data from
django_session table to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Jun 4, 2024
Fixes zulip#19490.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that model which results in more efficiency,
for example bulk deletion of sessions in "zerver/lib/sessions".

Create data_migration to copy over data from
django_session table to zerver_realmsession table.
abdelrahman725 added a commit to abdelrahman725/zulip that referenced this issue Jun 4, 2024
Fixes zulip#19490.

Create RealmSession model with custom fields: realm, user and ip_address,
indexed by (realm, user) and ip_address.

Migrate code to use that model which results in more efficiency,
for example bulk deletion of sessions in "zerver/lib/sessions".

Create data_migration to copy over data from
django_session table to zerver_realmsession table.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: performance Performance and scalability issues area: production
Projects
None yet
5 participants