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:
    • Rank:
      43903

      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.

        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: