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

Add Index for timemodified in mdl_grade_grade_history table

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide
      1. Upgrade your existing integration master site to the latest version and check that the grade_grades_history table has a key for the field timemodified.
      2. Create a fresh install of integration master and check the grade_grades_history table has a key for the field timemodified.
      Show
      Upgrade your existing integration master site to the latest version and check that the grade_grades_history table has a key for the field timemodified. Create a fresh install of integration master and check the grade_grades_history table has a key for the field timemodified.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-31557_master

      Description

      Currently timemodified isn't indexed and could cause performance issues when the following query is run during grade_history cleanup;

      DELETE FROM mdl_grade_grades_history WHERE timemodified < 1297008812

      This could be resolved by adding the following index;

      ALTER TABLE `mdl_grade_grades_history` ADD INDEX `mdl_gradgradhist_tim_ix` (`timemodified`)

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for the suggestion.

            Show
            salvetore Michael de Raadt added a comment - Thanks for the suggestion.
            Hide
            dakota.duff Dakota Duff added a comment -

            I'm getting similar with mdl_grade_grades_history table. An index is needed on timemodified. Also, I'm seeing this in the slow log:
            DELETE FROM mdl_grade_grades_history WHERE timemodified < '1259935181';

            I'm not positive, but it may also help if timemodified is passed as an integer, rather than a string. There shouldn't be any reason to pass as a string.

            Show
            dakota.duff Dakota Duff added a comment - I'm getting similar with mdl_grade_grades_history table. An index is needed on timemodified. Also, I'm seeing this in the slow log: DELETE FROM mdl_grade_grades_history WHERE timemodified < '1259935181'; I'm not positive, but it may also help if timemodified is passed as an integer, rather than a string. There shouldn't be any reason to pass as a string.
            Hide
            mchurch Mike Churchward added a comment -

            We can provide a patch for this, but what versions will you allow this to be updated in?

            Show
            mchurch Mike Churchward added a comment - We can provide a patch for this, but what versions will you allow this to be updated in?
            Hide
            andyjdavis Andrew Davis added a comment - - edited

            Hi Mike. You can make a patch available for whatever versions you see fit. Others can then apply the patch on their own. In terms of integration into the versions of Moodle from HQ however this fix would only go into the upcoming 2.4 as well as 2.3 and 2.2.

            Full details of the versions currently supported by HQ can be found at http://docs.moodle.org/dev/Releases

            Show
            andyjdavis Andrew Davis added a comment - - edited Hi Mike. You can make a patch available for whatever versions you see fit. Others can then apply the patch on their own. In terms of integration into the versions of Moodle from HQ however this fix would only go into the upcoming 2.4 as well as 2.3 and 2.2. Full details of the versions currently supported by HQ can be found at http://docs.moodle.org/dev/Releases
            Hide
            tgus Tim Gus added a comment -

            Currently working on this.

            Show
            tgus Tim Gus added a comment - Currently working on this.
            Hide
            jfilip Justin Filip added a comment - - edited

            Added testing instructions and link to where the code for this can be found.

            The work on this was done by Tim Gus here at Remote-Learner.

            Show
            jfilip Justin Filip added a comment - - edited Added testing instructions and link to where the code for this can be found. The work on this was done by Tim Gus here at Remote-Learner.
            Hide
            markn Mark Nelson added a comment -

            Hi Tim, patch looks good. Thanks for your contribution.

            Show
            markn Mark Nelson added a comment - Hi Tim, patch looks good. Thanks for your contribution.
            Hide
            markn Mark Nelson added a comment -

            Note to integrators, this is a database change so should only be applied to master.

            Show
            markn Mark Nelson added a comment - Note to integrators, this is a database change so should only be applied to master.
            Hide
            markn Mark Nelson added a comment -

            Hi Justin, I am assigning you to assignee field, and myself as the peer reviewer so that I am able to submit to integration. As an assignee on a tracker issue you are not supposed to be peer reviewer as well, even if you did not write the patch. This is in case any issues come back from the integrators and something needs addressing. If something does come back in this case and you are not able to fix it due to not having enough time (or whatever reason), I will take over as assignee and have someone else peer review it once I provide a fix. Thanks.

            Show
            markn Mark Nelson added a comment - Hi Justin, I am assigning you to assignee field, and myself as the peer reviewer so that I am able to submit to integration. As an assignee on a tracker issue you are not supposed to be peer reviewer as well, even if you did not write the patch. This is in case any issues come back from the integrators and something needs addressing. If something does come back in this case and you are not able to fix it due to not having enough time (or whatever reason), I will take over as assignee and have someone else peer review it once I provide a fix. Thanks.
            Hide
            damyon Damyon Wiese 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.

            Thanks!

            Show
            damyon Damyon Wiese 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. Thanks!
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Tim/Mark,

            I'm afraid this needs rebasing and the NEXT/PREVIOUS fields need removing as they were removed in MDL-37726.

            Mark is correct that we'll only acccept this on master. Cheers!

            Show
            poltawski Dan Poltawski added a comment - Hi Tim/Mark, I'm afraid this needs rebasing and the NEXT/PREVIOUS fields need removing as they were removed in MDL-37726 . Mark is correct that we'll only acccept this on master. Cheers!
            Hide
            cibot CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            markn Mark Nelson added a comment -

            Hi Dan, added another commit on top of the original to address this issue.

            Show
            markn Mark Nelson added a comment - Hi Dan, added another commit on top of the original to address this issue.
            Hide
            markn Mark Nelson added a comment -

            I also rebased the original, so that issue was addressed as well.

            Show
            markn Mark Nelson added a comment - I also rebased the original, so that issue was addressed as well.
            Hide
            markn Mark Nelson added a comment -

            Ugh, sorry for all the spam. Hopefully this will be the last email sent out due to my continuous editing of this issue. I decided against being the assignee and having someone else peer review this when the code had already been peer reviewed and the only change required was very minor on my behalf.

            Show
            markn Mark Nelson added a comment - Ugh, sorry for all the spam. Hopefully this will be the last email sent out due to my continuous editing of this issue. I decided against being the assignee and having someone else peer review this when the code had already been peer reviewed and the only change required was very minor on my behalf.
            Hide
            stronk7 Eloy Lafuente (stronk7) 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
            stronk7 Eloy Lafuente (stronk7) 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
            samhemelryk Sam Hemelryk added a comment -

            Thanks guys, this has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks guys, this has been integrated now.
            Hide
            abgreeve Adrian Greeve added a comment -

            I upgraded and created a new instance of moodle.
            The index appeared in both cases.
            Test passed.

            Show
            abgreeve Adrian Greeve added a comment - I upgraded and created a new instance of moodle. The index appeared in both cases. Test passed.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This is valid for unlimited entries to the, soon to be unveiled, Moodle Codebase Gardens. It includes free access to all facilities.

            Personal and non-transferable to all assignees, reviewers and testers in this issue. Valid until switching to Blackboard (100000€ penalization will be applied).

            Thanks, closing as fixed!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This is valid for unlimited entries to the, soon to be unveiled, Moodle Codebase Gardens. It includes free access to all facilities. Personal and non-transferable to all assignees, reviewers and testers in this issue. Valid until switching to Blackboard (100000€ penalization will be applied). Thanks, closing as fixed!

              People

              • Votes:
                3 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

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