Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-44908

Slow upgrade_grade_item_fix_sortorder

    Details

    • Testing Instructions:
      Hide

      Follow testing instructions from MDL-41062 (you will need an initial instance older than 23/Jan/14 )

      Show
      Follow testing instructions from MDL-41062 (you will need an initial instance older than 23/Jan/14 )
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE, MOODLE_27_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:

      Description

      The upgrade_grade_item_fix_sortorder function is still much too slow, even after MDL-43946.

      On one of our sites, it appears this will take at least several hours to finish (I didn't let it go, just guessing from what progress it made after 30 minutes...) all the while blocking much happening with grade items.

      The problem appears to be that when courses have a large amount of grade items (in our case some with 4000 and more) the related rows are updated thousands of times; besides the O(N*N) time complexity there is a huge bloating of the table during the transaction which causes its own problems.

      My patch moves this update loop for a given course to memory in PHP, then writes the updates back out when that course is finished. This results in only O(N) rows updated, and completes in a few seconds on a problematic dataset.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            tlevi Tony Levi added a comment -

            Adding patch link.

            Show
            tlevi Tony Levi added a comment - Adding patch link.
            Hide
            tlevi Tony Levi added a comment -

            Ugh, patch isn't right, back to the drawing board.

            Show
            tlevi Tony Levi added a comment - Ugh, patch isn't right, back to the drawing board.
            Hide
            tlevi Tony Levi added a comment -

            Fixed, it appears = is not the same as == _.

            Show
            tlevi Tony Levi added a comment - Fixed, it appears = is not the same as == _ .
            Hide
            marina Marina Glancy added a comment -

            Thanks Tony, I'm requesting peer review on your behalf

            Show
            marina Marina Glancy added a comment - Thanks Tony, I'm requesting peer review on your behalf
            Hide
            cibot CiBoT added a comment -

            Results for MDL-44908

            Show
            cibot CiBoT added a comment - Results for MDL-44908 Remote repository: https://github.com/tlevi/moodle Remote branch mdl44908 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2684 Warning: The mdl44908 branch at https://github.com/tlevi/moodle has not been rebased recently (>20 days ago). Details: http://integration.moodle.org/job/Precheck%20remote%20branch/2684/artifact/work/smurf.html
            Hide
            poltawski Dan Poltawski added a comment -

            Sending all 'waiting for peer review' issues for integration review. The integration team are doing this to ensure any 'integratable issues' will not got missed for freeze.

            Note: We will prioritise peer reviewed issues and may not spend as much time examining non-integratable, non peer-reviewed issues.

            This is a present from the iTeam - it means that peer review is not working well enough! We really do not want to do this again! Lets improve our peer review process!

            Show
            poltawski Dan Poltawski added a comment - Sending all 'waiting for peer review' issues for integration review. The integration team are doing this to ensure any 'integratable issues' will not got missed for freeze. Note: We will prioritise peer reviewed issues and may not spend as much time examining non-integratable, non peer-reviewed issues. This is a present from the iTeam - it means that peer review is not working well enough! We really do not want to do this again! Lets improve our peer review process!
            Hide
            damyon Damyon Wiese added a comment -

            Thanks - still looking at this. I just want to do some cross DB testing on this.

            Show
            damyon Damyon Wiese added a comment - Thanks - still looking at this. I just want to do some cross DB testing on this.
            Hide
            damyon Damyon Wiese added a comment -

            Attached my test script to create 4000 assignments in course 2 - then set the sortorder for all their gradeitems to 1.

            Show
            damyon Damyon Wiese added a comment - Attached my test script to create 4000 assignments in course 2 - then set the sortorder for all their gradeitems to 1.
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Tony,

            I've tested this on all databases with a course with 4000 assignments and all set to have the same sortorder in the gradebook. All passing so this has been integrated to 25, 26 and master.

            Show
            damyon Damyon Wiese added a comment - Thanks Tony, I've tested this on all databases with a course with 4000 assignments and all set to have the same sortorder in the gradebook. All passing so this has been integrated to 25, 26 and master.
            Hide
            damyon Damyon Wiese added a comment -

            I tested this all day in integration - so passing.

            Show
            damyon Damyon Wiese added a comment - I tested this all day in integration - so passing.
            Hide
            tlevi Tony Levi added a comment -

            Neat; FWIW its been run against a few of our large sets now and has worked nice and quick, hopefully this is really buried at last.

            Show
            tlevi Tony Levi added a comment - Neat; FWIW its been run against a few of our large sets now and has worked nice and quick, hopefully this is really buried at last.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many thanks! Your awesome code is now upstream!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks! Your awesome code is now upstream!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/May/14