Moodle
  1. Moodle
  2. MDL-35252

Removing a grade override on an assignment (2.3) grade creates a mdl_grade_grades row for all users in mdl_user for that grade item

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3.1
    • Fix Version/s: 2.3.3
    • Component/s: Assignment, Gradebook
    • Labels:
      None
    • Testing Instructions:
      Hide
      • Create a new course
      • Enrol 1 user in the course
      • Create an assignment in the course
      • View the course gradebook
      • Edit the grade for the user in the assignment - (do not make any changes - leave "overridden" off but save the form).
      • Run this SQL

        select count(*) from mdl_grade_grades where itemid in (select MAX(id) from mdl_grade_items);

      • The answer should be 1
      Show
      Create a new course Enrol 1 user in the course Create an assignment in the course View the course gradebook Edit the grade for the user in the assignment - (do not make any changes - leave "overridden" off but save the form). Run this SQL select count(*) from mdl_grade_grades where itemid in (select MAX(id) from mdl_grade_items); The answer should be 1
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      Removing grade override on an assignment (2.3) grade creates a row in mdl_grade_grades for all users in mdl_user table for that grade item. This is causing performance problems on sites with a large number of users. For sites with a large number of users this will cause the script to run out of memory and never completely finish.

      To reproduce:
      1. Run this query to see how many mdl_grade_grades the guest user has:
      SELECT COUNT FROM mdl_grade_grades WHERE userid = 1 LIMIT 1;
      2. Create a course.
      3. Add an assignment (2.3 not older 2.2)
      4. Add 1 user as a student to course, but not guest user.
      5. Turn on editing in gradebook and type a grade in the assignment and update.
      6. Edit the grade and uncheck the "Overridden" check box and save changes. For sites with a large number of users this will cause the script to run out of memory.
      7. Run this query to see how many mdl_grade_grades the guest user has:
      SELECT COUNT FROM mdl_grade_grades WHERE userid = 1 LIMIT 1;
      You will see that count has increased.

      Running
      SELECT userid, COUNT FROM mdl_grade_grades WHERE itemid IN (SELECT itemid FROM mdl_grade_grades WHERE userid = 1) GROUP BY userid ORDER BY userid LIMIT 30;
      will help convince you that it is creating rows for all users for that itemid.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Michael Spall added a comment -

            I have tested this on Moodle 2.3.1+ (Build: 20120831)

            Show
            Michael Spall added a comment - I have tested this on Moodle 2.3.1+ (Build: 20120831)
            Hide
            Michael Spall added a comment -

            I tested this with assignments 2.2, forums, and quizzes and those activities didn't have this behavior.

            Show
            Michael Spall added a comment - I tested this with assignments 2.2, forums, and quizzes and those activities didn't have this behavior.
            Hide
            Raymond Antonio added a comment -

            Hi Damyon and Michael,

            This is the patch for the issue and it is ready for testing. The patch's details are as follows:
            1. Repo
            git://github.com/raymondAntonio/moodle.git
            2. The branch
            MDL-35252
            And This is the diff :
            https://github.com/raymondAntonio/moodle/commit/2eb4eebe1f6c91a470df0d090bdc7926550a959e

            Cheers

            Show
            Raymond Antonio added a comment - Hi Damyon and Michael, This is the patch for the issue and it is ready for testing. The patch's details are as follows: 1. Repo git://github.com/raymondAntonio/moodle.git 2. The branch MDL-35252 And This is the diff : https://github.com/raymondAntonio/moodle/commit/2eb4eebe1f6c91a470df0d090bdc7926550a959e Cheers
            Hide
            Damyon Wiese added a comment - - edited

            Hi Raymond this fix is not correct and results in duplicate records in the gradebook. I'll take a look and find the cause of this issue. I am still checking this fix.

            Thanks, Damyon

            Show
            Damyon Wiese added a comment - - edited Hi Raymond this fix is not correct and results in duplicate records in the gradebook. I'll take a look and find the cause of this issue . I am still checking this fix. Thanks, Damyon
            Hide
            Damyon Wiese added a comment -

            Hi Raymond,

            Yes this fix is OK - changing this to a left join will mean that only users that have been given a grade for this assignment will be returned. I will create a separate issue to not return any results in blind marking is enabled as I spotted that while reviewing this code.

            Thanks, Damyon

            Show
            Damyon Wiese added a comment - Hi Raymond, Yes this fix is OK - changing this to a left join will mean that only users that have been given a grade for this assignment will be returned. I will create a separate issue to not return any results in blind marking is enabled as I spotted that while reviewing this code. Thanks, Damyon
            Hide
            Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            Raymond Antonio added a comment -

            Hi Dan
            Rebase done. Cheers

            Show
            Raymond Antonio added a comment - Hi Dan Rebase done. Cheers
            Hide
            Dan Poltawski added a comment -

            Does this fix need back porting?

            Its a shame we don't have unit tests for this, i'd be happier about it if we had tests in this scenario

            Show
            Dan Poltawski added a comment - Does this fix need back porting? Its a shame we don't have unit tests for this, i'd be happier about it if we had tests in this scenario
            Hide
            Dan Poltawski added a comment -

            Ping Raymond/Damyon? Should this be backported?

            Show
            Dan Poltawski added a comment - Ping Raymond/Damyon? Should this be backported?
            Hide
            Damyon Wiese added a comment -

            Yes this fix needs backporting, Raymond can you add a 2.3 branch to this issue? (I checked and the commit cherry-picks fine)

            Show
            Damyon Wiese added a comment - Yes this fix needs backporting, Raymond can you add a 2.3 branch to this issue? (I checked and the commit cherry-picks fine)
            Hide
            Raymond Antonio added a comment -

            Hi Damyon and Dan,

            2.3 branch has been added. Cheers

            Show
            Raymond Antonio added a comment - Hi Damyon and Dan, 2.3 branch has been added. Cheers
            Hide
            Dan Poltawski added a comment -

            Thanks guys, i've integrated this now.

            Show
            Dan Poltawski added a comment - Thanks guys, i've integrated this now.
            Hide
            Michael de Raadt added a comment -

            This would have been better if it were tested by a unit test.

            Show
            Michael de Raadt added a comment - This would have been better if it were tested by a unit test.
            Hide
            Rajesh Taneja added a comment -

            Thanks Raymond and Damyon.
            Only one record is created.

            Show
            Rajesh Taneja added a comment - Thanks Raymond and Damyon. Only one record is created.
            Hide
            Dan Poltawski added a comment -

            Congratulations, you've done it!

            Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc

            Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

            Show
            Dan Poltawski added a comment - Congratulations, you've done it! Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

              People

              • Votes:
                11 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: