Skip to content

Speed up deleting of global user achievements. #2710

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 1 commit into
base: develop
Choose a base branch
from

Conversation

drgrice1
Copy link
Member

Instead of listing user achievements and then deleting them one by one in a loop, just delete them with one query using a where statement.

This improved the deletion of 5000 users all of which had the default set of achievements assigned to them from taking a minute and a half to less than 20 seconds.

@drgrice1
Copy link
Member Author

You can test this using the class list provided in #2709 and in combination with testing that pull request. Delete the users from imported and for which the achievements are assigned for testing that pull request.

drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request Apr 20, 2025
If you have payed attention (or tested openwebwork#2709 and openwebwork#2710), then you know
that this takes a long time if there are a large number of users.  Way
to long to be at all reasonable.

This uses techniques like those used for assigning multiple sets to
users, i.e., the `WeBWorK::DB::Schema::NewSQL::Std::insert_records` and
`WeBWorK::DB::Schema::NewSQL::Std::put_records` methods, to speed up
assignment of achievements to users.

This brings the assignment time for 5000 users down from more than 8
minutes, to less than 30 seconds.
drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request Apr 20, 2025
If you have payed attention (or tested openwebwork#2709 and openwebwork#2710), then you know
that this takes a long time if there are a large number of users.  Way
to long to be at all reasonable.

This uses techniques like those used for assigning multiple sets to
users, i.e., the `WeBWorK::DB::Schema::NewSQL::Std::insert_records` and
`WeBWorK::DB::Schema::NewSQL::Std::update_records` methods, to speed up
assignment of achievements to users.

This brings the assignment time for 5000 users down from more than 8
minutes, to less than 30 seconds.
drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request Apr 20, 2025
If you have payed attention (or tested openwebwork#2709 and openwebwork#2710), then you know
that this takes a long time if there are a large number of users.  Way
to long to be at all reasonable.

This uses techniques like those used for assigning multiple sets to
users, i.e., the `WeBWorK::DB::Schema::NewSQL::Std::insert_records` and
`WeBWorK::DB::Schema::NewSQL::Std::update_records` methods, to speed up
assignment of achievements to users.

This brings the assignment time for 5000 users down from more than 8
minutes, to less than 30 seconds.
drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request Apr 20, 2025
If you have payed attention (or tested openwebwork#2709 and openwebwork#2710), then you know
that this takes a long time if there are a large number of users.  Way
to long to be at all reasonable.

This uses techniques like those used for assigning multiple sets to
users, i.e., the `WeBWorK::DB::Schema::NewSQL::Std::insert_records` and
`WeBWorK::DB::Schema::NewSQL::Std::update_records` methods, to speed up
assignment of achievements to users.

This brings the assignment time for 5000 users down from more than 8
minutes, to less than 30 seconds.
drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request Apr 20, 2025
If you have payed attention (or tested openwebwork#2709 and openwebwork#2710), then you know
that this takes a long time if there are a large number of users.  Way
to long to be at all reasonable.

This uses techniques like those used for assigning multiple sets to
users, i.e., the `WeBWorK::DB::Schema::NewSQL::Std::insert_records` and
`WeBWorK::DB::Schema::NewSQL::Std::update_records` methods, to speed up
assignment of achievements to users.

This brings the assignment time for 5000 users down from more than 8
minutes, to less than 30 seconds.
@Alex-Jordan
Copy link
Contributor

The new line 1471 assumes $self->{global_user_achievement} will always be something that the delete method applies to. Is it for sure that there is no scenario where $self->{global_user_achievement} is simply undefined? (I am not up to speed on how the global_user_achievement property is initialized.)

@drgrice1
Copy link
Member Author

All of the hash keys of $self are references to WeBWorK::DB::Schema objects. They are defined in the WeBWorK::DB::new method earlier in this file (or really the WeBWorK::DB::init_table method call by new), and correspond to the hash keys of the database layout. So if you are using an alternate database layout that does not have a global_user_achievement table defined, then that would be undefined. Then all of the methods from lines 1406 through 1465 that don't check to see $self->{global_user_achievement} is undefined will fail with an exception. However, this method will succeed and always return 0 in that case. So attempting to count, list, check exists, get, add, or put a global user achievement all will throw an exception if you don't have global_user_achievement defined in your database layout, but deleting a global user achievement is okay.

Basically this method is entirely inconsistent with every other method in this file. If this were consistent, then every method in this file should perform this same check to see that the hash key for the corresponding schema is defined before doing anything.

Since we do not support alternate database layouts anymore, and furthermore, since there won't even be the option to have an alternate database layout soon (once #2702 is merged), this check is just not needed.

drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request Apr 22, 2025
If you have payed attention (or tested openwebwork#2709 and openwebwork#2710), then you know
that this takes a long time if there are a large number of users.  Way
to long to be at all reasonable.

This uses techniques like those used for assigning multiple sets to
users, i.e., the `WeBWorK::DB::Schema::NewSQL::Std::insert_records` and
`WeBWorK::DB::Schema::NewSQL::Std::update_records` methods, to speed up
assignment of achievements to users.

This brings the assignment time for 5000 users down from more than 8
minutes, to less than 30 seconds.
@drgrice1 drgrice1 force-pushed the delete-global-user-achievement-speed-tweak branch from c6c6ba3 to 2943b31 Compare April 22, 2025 10:31
Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

See the speed up.

drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request Apr 27, 2025
If you have payed attention (or tested openwebwork#2709 and openwebwork#2710), then you know
that this takes a long time if there are a large number of users.  Way
to long to be at all reasonable.

This uses techniques like those used for assigning multiple sets to
users, i.e., the `WeBWorK::DB::Schema::NewSQL::Std::insert_records` and
`WeBWorK::DB::Schema::NewSQL::Std::update_records` methods, to speed up
assignment of achievements to users.

This brings the assignment time for 5000 users down from more than 8
minutes, to less than 30 seconds.
@drgrice1 drgrice1 force-pushed the delete-global-user-achievement-speed-tweak branch from 2943b31 to 9d37488 Compare April 27, 2025 16:17
Instead of listing user achievements and then deleting them one by one
in a loop, just delete them with one query using a `where` statement.
@drgrice1 drgrice1 force-pushed the delete-global-user-achievement-speed-tweak branch from 9d37488 to ebd2e99 Compare April 27, 2025 16:21
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.

3 participants