Improve database usage by consolidating and removing database calls by drgrice1 · Pull Request #1585 · openwebwork/webwork2

drgrice1 · 2022-02-08T19:05:49Z

Most of the DBFIXME's scattered throughout the code have been dealt with. In some cases this was accomplished by doing exactly what was said. In some cases better approaches were used to consolidate calls and cache data.

In other cases the DBFIXME's were just deleted. The primary case of this is those asking for the usage of database iterators. Those are really not correct. The amount of data contained in the database queries is generally not substantial enough to justify their usage. Furthermore, the correct usage of database iteration (not implemented in webwork) would accumulate data into chunks of a more significant size rather than handle each row separately.

WHERE clauses are used to obtain records, rather than listing id's and then filtering on those id's. ORDER BY clauses are used to do sorting in the database, rather than sorting in perl.

This usage has been tested on a course that I have that has more than 40,000 users. Significant speed improvements are seen particularly on the pages that utilize the data for all users, like the classlist editor, statistics, and student progress pages.

Note that now the set level proctor users are not visible anywhere in the user interface. Previously they were hidden on the classlist editor page, but showed up many places where they should not have. There isn't anywhere that they should be shown.

Also note that several invalid e flags on regex expressions have been removed in Grades.pm and SendMail.pm. There was a FIXME in Grades.pm stating that this flag is not needed for variable interpolation, and it isn't. I have seen also seen errors due to these flags being used.

I recommend using the "Hide Whitespace" option to view this (if you haven't become accustomed to this already).

I also added a .perltidyrc file and ran perltidy on all of the files in the directory lib/WeBWorK/DB. There are no operational changes to those files, so just ignore them in reviewing this pull request. The pull request is currently in two commits. The first has all of the real changes, and the second the perltidy stuff. (Note that the .perltidyrc file is the same as the webwork3 file except I added the -xci option. We may want to add that to webwork3's file and the pg one also. That does a much better job of indentation with nested structures.)

This is #1583 again. @pstaabp be more careful!

lib/WeBWorK/DB/. Also update the copyright years and project url, and delete the CVSHeader in those files.

Remove the protection in UserDetail.pm against showing more than 150 sets. That protection wasn't working.

drdrew42 · 2022-02-08T22:42:38Z

I'm getting an error on the Email page (SendMail.pm L#138-47):

DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'AND ( `user_id` NOT LIKE 'set_id:%' AND `user_id` NOT LIKE 'practice%' ) ) ) ORD' at line 1 at /opt/webwork/webwork2/lib/WeBWorK/DB/Schema/NewSQL/Std.pm line 881.

Which is somewhat strange, given that a similar WHERE clause is used elsewhere (e.g. StudentProgress) and I don't see errors there.

drgrice1 · 2022-02-08T22:46:17Z

It isn't odd. You don't have viewable_sections or viewable_recitations arrays defined, and I forgot to add the protection about that here. I added it on line 350 of StudentProgress.pm, but forgot it here.

drgrice1 · 2022-02-08T22:48:54Z

Thanks for testing and finding that. It is fixed now.

drdrew42 · 2022-02-08T23:05:17Z

I think you committed some other changes ... ceed876 modifies UserDetail.pm

drgrice1 · 2022-02-08T23:06:49Z

Oops. Did not want git commit -a.

drgrice1 · 2022-02-08T23:07:53Z

Ahh, I didn't use git commit -a. I just added the wrong file.

drgrice1 · 2022-02-08T23:08:23Z

Also, I force pushed that fix.

drdrew42 · 2022-02-08T23:12:26Z

That worked, the email page now loads without error. 👍

are not defined, then the or in the where clause is not added.

that are like the versioned naming scheme in ProblemSetDetail.pm.

makes it possible to delete quiz versions (manually in the database at this point), and the table will still properly display all existing versions even if the numbering is not consecutive.

This has been requested in the forums (see for instance https://webwork.maa.org/moodle/mod/forum/discuss.php?d=2400), and is something that I would also find useful. In my classes I have quizzes that allow three versions with one submission per version, and show only one problem per page. Students frequently will start one version, and then accidentally hit "Grade Test" after only working one problem. This wastes one of the versions for this student. Prior to this pull request I could do this, but had to do so manually by deleting the version and problems in the database. Note that doing so also does not cause problems anywhere else. Note that prior to pull request openwebwork#1585 there was a minor glitch caused on the user detail page if a version was deleted that was not the last version that exists. (This frequently happens because not only do students accidentally hit submit, but then they go start another version before contacting me to fix the problem.) The glitch was caused by the assumption that the version_id stored in the database coincided with array indices (offset by one) in the array of versioned user sets obtained by listing all versions in the database. That of course is not the case with openwebwork#1585. I have been deleting versions manually for years with no issues other than this one. This adds a checkbox to the quiz versions on the user detail page (which you can get to by clicking on the assigned sets for a user in the class list editor). If this checkbox is unchecked, then that specific version of the gateway quiz will be deleted for that user. Note that javascript is used to make the server side behavior clear to the user. If the template set is unchecked, then all of the versions are unchecked as in addition to the template set being unassigned, the versions will all be deleted. If a version is checked (after previously being unchecked) then it is ensured that the template set is also checked. Since this adds a javascript file for the UserDetail.pm module, all of the inline javascript in that file is also moved into the javascript file (the change, blur, and keyup handlers for the date inputs, and the click handler for the "Assign All Sets to Current User" button).

…rovements

This has been requested in the forums (see for instance https://webwork.maa.org/moodle/mod/forum/discuss.php?d=2400), and is something that I would also find useful. In my classes I have quizzes that allow three versions with one submission per version, and show only one problem per page. Students frequently will start one version, and then accidentally hit "Grade Test" after only working one problem. This wastes one of the versions for this student. Prior to this pull request I could do this, but had to do so manually by deleting the version and problems in the database. Note that doing so also does not cause problems anywhere else. Note that prior to pull request openwebwork#1585 there was a minor glitch caused on the user detail page if a version was deleted that was not the last version that exists. (This frequently happens because not only do students accidentally hit submit, but then they go start another version before contacting me to fix the problem.) The glitch was caused by the assumption that the version_id stored in the database coincided with array indices (offset by one) in the array of versioned user sets obtained by listing all versions in the database. That of course is not the case with openwebwork#1585. I have been deleting versions manually for years with no issues other than this one. This adds a checkbox to the quiz versions on the user detail page (which you can get to by clicking on the assigned sets for a user in the class list editor). If this checkbox is unchecked, then that specific version of the gateway quiz will be deleted for that user. Note that javascript is used to make the server side behavior clear to the user. If the template set is unchecked, then all of the versions are unchecked as in addition to the template set being unassigned, the versions will all be deleted. If a version is checked (after previously being unchecked) then it is ensured that the template set is also checked. Since this adds a javascript file for the UserDetail.pm module, all of the inline javascript in that file is also moved into the javascript file (the change, blur, and keyup handlers for the date inputs, and the click handler for the "Assign All Sets to Current User" button).

the same way on the UserDetail.pm page. The alphabetic sort use for the merged set versions of course will not match the numeric sort used for the set versions. If only the merged set versions were implemented in the same way as the set versions...

This has been requested in the forums (see for instance https://webwork.maa.org/moodle/mod/forum/discuss.php?d=2400), and is something that I would also find useful. In my classes I have quizzes that allow three versions with one submission per version, and show only one problem per page. Students frequently will start one version, and then accidentally hit "Grade Test" after only working one problem. This wastes one of the versions for this student. Prior to this pull request I could do this, but had to do so manually by deleting the version and problems in the database. Note that doing so also does not cause problems anywhere else. Note that prior to pull request openwebwork#1585 there was a minor glitch caused on the user detail page if a version was deleted that was not the last version that exists. (This frequently happens because not only do students accidentally hit submit, but then they go start another version before contacting me to fix the problem.) The glitch was caused by the assumption that the version_id stored in the database coincided with array indices (offset by one) in the array of versioned user sets obtained by listing all versions in the database. That of course is not the case with openwebwork#1585. I have been deleting versions manually for years with no issues other than this one. This adds a checkbox to the quiz versions on the user detail page (which you can get to by clicking on the assigned sets for a user in the class list editor). If this checkbox is unchecked, then that specific version of the gateway quiz will be deleted for that user. Note that javascript is used to make the server side behavior clear to the user. If the template set is unchecked, then all of the versions are unchecked as in addition to the template set being unassigned, the versions will all be deleted. If a version is checked (after previously being unchecked) then it is ensured that the template set is also checked. Since this adds a javascript file for the UserDetail.pm module, all of the inline javascript in that file is also moved into the javascript file (the change, blur, and keyup handlers for the date inputs, and the click handler for the "Assign All Sets to Current User" button).

pstaabp · 2022-02-10T22:48:28Z

Overall, this looks good. I'm trying to test everything, since this seems to touch nearly every part of webwork.

Since you mention speed up, I've noticed that the Homework Sets Editor still loads slowly compared to others and I only really notice it when I have caching turned off. I haven't dug into what might be causing it. Was there anything in here that should speed this up?

drgrice1 · 2022-02-10T22:52:44Z

I have noticed that as well. I am still looking into that. The thing is that that page loads more data from the database than any other. It doesn't do so very efficiently, but to make it so will require quite the rewrite.

This has been requested in the forums (see for instance https://webwork.maa.org/moodle/mod/forum/discuss.php?d=2400), and is something that I would also find useful. In my classes I have quizzes that allow three versions with one submission per version, and show only one problem per page. Students frequently will start one version, and then accidentally hit "Grade Test" after only working one problem. This wastes one of the versions for this student. Prior to this pull request I could do this, but had to do so manually by deleting the version and problems in the database. Note that doing so also does not cause problems anywhere else. Note that prior to pull request openwebwork#1585 there was a minor glitch caused on the user detail page if a version was deleted that was not the last version that exists. (This frequently happens because not only do students accidentally hit submit, but then they go start another version before contacting me to fix the problem.) The glitch was caused by the assumption that the version_id stored in the database coincided with array indices (offset by one) in the array of versioned user sets obtained by listing all versions in the database. That of course is not the case with openwebwork#1585. I have been deleting versions manually for years with no issues other than this one. This adds a checkbox to the quiz versions on the user detail page (which you can get to by clicking on the assigned sets for a user in the class list editor). If this checkbox is unchecked, then that specific version of the gateway quiz will be deleted for that user. Note that javascript is used to make the server side behavior clear to the user. If the template set is unchecked, then all of the versions are unchecked as in addition to the template set being unassigned, the versions will all be deleted. If a version is checked (after previously being unchecked) then it is ensured that the template set is also checked. Since this adds a javascript file for the UserDetail.pm module, all of the inline javascript in that file is also moved into the javascript file (the change, blur, and keyup handlers for the date inputs, and the click handler for the "Assign All Sets to Current User" button).

drdrew42

Everything seems to be running smoothly

dlglin · 2022-02-11T17:03:23Z

Since you mention speed up, I've noticed that the Homework Sets Editor still loads slowly compared to others and I only really notice it when I have caching turned off. I haven't dug into what might be causing it. Was there anything in here that should speed this up?

Every time the Homework Sets Editor loads it combs through the entire templates directory tree looking for def files, including the OPL and Contrib. Depending on how fast your storage is this can greatly slow down the page load.

drgrice1 · 2022-02-11T17:08:02Z

@dlglin: I just did some timing of things, and came to exactly that conclusion. It is not a database query, it is line 1269 of ProblemSetList.pm that is causing the slow down, and that is the line that does what you say. That is the line number it is on in this pull request (not develop).

sorting in the database. This was not included in openwebwork#1585 because I didn't want to cause merge conflicts with openwebwork#1569. The file is also cleaned up a bit and the gateway versions listing and homework problem listing sections separated in a more logical and readable way. Those sections were perltidied.

@taniwallach

… sets. This was a regression introduced in openwebwork#1585. I missed a place where the result of a list...Where method was used and needed to be mapped to the actual field value. I double checked, and this is the only place this was forgotten. Thanks @taniwallach for pointing this out.

@taniwallach

… sets. This was a regression introduced in openwebwork#1585. I missed a place where the result of a list...Where method was used and needed to be mapped to the actual field value. I double checked, and this is the only place this was forgotten. Thanks @taniwallach for pointing this out.

sorting in the database. This was not included in openwebwork#1585 because I didn't want to cause merge conflicts with openwebwork#1569. The file is also cleaned up a bit and the gateway versions listing and homework problem listing sections separated in a more logical and readable way. Those sections were perltidied.

drgrice1 added 3 commits February 8, 2022 13:03

Improve some of the database calls.

25b95b9

Add a .perltidyrc file, and run perltidy on all of the .pm files in

6ed3a7d

lib/WeBWorK/DB/. Also update the copyright years and project url, and delete the CVSHeader in those files.

Fix a typo that was cut an pasted into numerous places.

acc5a45

Remove the protection in UserDetail.pm against showing more than 150 sets. That protection wasn't working.

drgrice1 mentioned this pull request Feb 8, 2022

Improve database usage by consolidating and removing database calls #1583

Merged

drgrice1 force-pushed the database-usage-improvements branch from ceed876 to 9a6c4b4 Compare February 8, 2022 23:06

Make sure that if the viewable_sections and viewable_recitations arrays

bb18417

are not defined, then the or in the where clause is not added.

drgrice1 force-pushed the database-usage-improvements branch 2 times, most recently from c47544a to c3e3d9a Compare February 9, 2022 02:34

The getMergedProblemVersionsWhere method needs to select on the set_id's

0da992d

that are like the versioned naming scheme in ProblemSetDetail.pm.

drgrice1 force-pushed the database-usage-improvements branch from c3e3d9a to 0da992d Compare February 9, 2022 02:41

drgrice1 added 2 commits February 8, 2022 21:04

Clean up the generation of the set list on the UserDetail.pm page. This

88ceb81

makes it possible to delete quiz versions (manually in the database at this point), and the table will still properly display all existing versions even if the numbering is not consecutive.

Remove another get list then get records instance.

5588d8d

drgrice1 mentioned this pull request Feb 9, 2022

New feature: Add the capability of deleting a specific quiz version. #1586

Merged

Merge remote-tracking branch 'origin/develop' into database-usage-imp…

31f5cf5

…rovements

drgrice1 force-pushed the database-usage-improvements branch from 2003c7f to 55bc9fa Compare February 11, 2022 12:34

drdrew42 approved these changes Feb 11, 2022

View reviewed changes

pstaabp merged commit 8b9db85 into openwebwork:develop Feb 11, 2022

drgrice1 deleted the database-usage-improvements branch February 11, 2022 21:17

somiaj mentioned this pull request Feb 12, 2022

Assigning homework sets display proctor set users. #1565

Closed

drgrice1 mentioned this pull request Apr 1, 2022

Remove some more redundant database calls in ProblemSet.pm and add sorting in the database. #1642

Merged

drgrice1 mentioned this pull request Apr 6, 2022

Fix the filter on the ProblemSetList.pm for showing visible or hidden sets. #1650

Merged

taniwallach mentioned this pull request Apr 6, 2022

Change the wording in the Filter, Edit, and Export selects as suggested in issue #1480. #1646

Merged

drgrice1 mentioned this pull request Apr 7, 2022

Improve DB code - use iterators, transactions #1244

Open

Uh oh!

Conversation

drgrice1 commented Feb 8, 2022

Uh oh!

drdrew42 commented Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drgrice1 commented Feb 8, 2022

Uh oh!

drgrice1 commented Feb 8, 2022

Uh oh!

drdrew42 commented Feb 8, 2022

Uh oh!

drgrice1 commented Feb 8, 2022

Uh oh!

drgrice1 commented Feb 8, 2022

Uh oh!

drgrice1 commented Feb 8, 2022

Uh oh!

drdrew42 commented Feb 8, 2022

Uh oh!

pstaabp commented Feb 10, 2022

Uh oh!

drgrice1 commented Feb 10, 2022

Uh oh!

drdrew42 left a comment

Choose a reason for hiding this comment

Uh oh!

dlglin commented Feb 11, 2022

Uh oh!

drgrice1 commented Feb 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

drdrew42 commented Feb 8, 2022 •

edited

Loading

drgrice1 commented Feb 11, 2022 •

edited

Loading