Details

    • Database:
      Any
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      Adding userid and fieldid key to increase the performence of SGBD.

      <KEY NAME="userid" TYPE="unique" FIELDS="userid" PREVIOUS="primary" NEXT="fieldid"/>
      <KEY NAME="fieldid" TYPE="unique" FIELDS="fieldid" PREVIOUS="userid"/>

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              karakayalardanmine@gmail.com Mine Karakaya added a comment -

              Hi, I am planning to do XML admin settings project for my SOC. I have added a patch for this bug as attachment.

              Show
              karakayalardanmine@gmail.com Mine Karakaya added a comment - Hi, I am planning to do XML admin settings project for my SOC. I have added a patch for this bug as attachment.
              Hide
              colin Colin Campbell added a comment -

              We were getting very poor performance on selects on user_info_data, so we definitely have a need for an index on the userid, but I am not sure what benefit the separate index on fieldid would provide. Do you have any queries in mind that you think the fieldid index would help? I am generally conservative about adding indexes, so if no specific benefit is expected, it should not be added.

              Also, neither index as given in the description can be unique.

              I added the following to my local/db/upgrade.php.

              if ($result && $oldversion < 2009060600)

              { $table = new XMLDBTable('user_info_data'); $index = new XMLDBIndex('userid-fieldid'); $index->setAttributes(XMLDB_INDEX_NOTUNIQUE, array('userid', 'fieldid')); $result = $result && add_index($table, $index); }

              Note that this is an index on both userid and fieldid. Although the code for user_info_data inserts implies that the combination of userid and fieldid are unique, I chose not to add such a constraint myself because the benefit would not be great and I would run the risk of the moodle.org code later breaking on my constraint. If we can get this index on both columns added to the core, however, I suggest that it be added as a unique index.

              Show
              colin Colin Campbell added a comment - We were getting very poor performance on selects on user_info_data, so we definitely have a need for an index on the userid, but I am not sure what benefit the separate index on fieldid would provide. Do you have any queries in mind that you think the fieldid index would help? I am generally conservative about adding indexes, so if no specific benefit is expected, it should not be added. Also, neither index as given in the description can be unique. I added the following to my local/db/upgrade.php. if ($result && $oldversion < 2009060600) { $table = new XMLDBTable('user_info_data'); $index = new XMLDBIndex('userid-fieldid'); $index->setAttributes(XMLDB_INDEX_NOTUNIQUE, array('userid', 'fieldid')); $result = $result && add_index($table, $index); } Note that this is an index on both userid and fieldid. Although the code for user_info_data inserts implies that the combination of userid and fieldid are unique, I chose not to add such a constraint myself because the benefit would not be great and I would run the risk of the moodle.org code later breaking on my constraint. If we can get this index on both columns added to the core, however, I suggest that it be added as a unique index.
              Hide
              chuang Wen Hao Chuang added a comment -

              +1 for this. We found the same thing here at SFSU too.

              Show
              chuang Wen Hao Chuang added a comment - +1 for this. We found the same thing here at SFSU too.
              Hide
              jfilip Justin Filip added a comment -

              +1

              At Remote Learner this problem was causing a database to completely lock up when using the browse users filters and including searching on a custom profile field.

              Show
              jfilip Justin Filip added a comment - +1 At Remote Learner this problem was causing a database to completely lock up when using the browse users filters and including searching on a custom profile field.
              Hide
              mchurch Mike Churchward added a comment -

              Just added a new issue about this - linked to this one. Although it duplicates this issue, the solution was a bit different - one index instead of two. Maybe two is better? Or maybe three?

              Show
              mchurch Mike Churchward added a comment - Just added a new issue about this - linked to this one. Although it duplicates this issue, the solution was a bit different - one index instead of two. Maybe two is better? Or maybe three?
              Hide
              maherne Michael Aherne added a comment -

              +1

              We had the same problem with the database locking when filtering users by a profile field (although the tabletype was MyISAM at that point).

              A single index on userid & fieldid together should fix performance for code using the /user/profile/lib.php library, but I think another index on userid alone would be useful too for anyone trying to get a user's profile data in a single SQL query.

              Show
              maherne Michael Aherne added a comment - +1 We had the same problem with the database locking when filtering users by a profile field (although the tabletype was MyISAM at that point). A single index on userid & fieldid together should fix performance for code using the /user/profile/lib.php library, but I think another index on userid alone would be useful too for anyone trying to get a user's profile data in a single SQL query.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Moving to stable backlog, randomly assigning to Andrew and raising to critical to be included in next performance-focussed sprint @ HQ.

              Some notes:

              1) Take a look to MDL-24695 (already assigned to Andrew). That shows the correct solution: one non-unique index by userid and fieldid.
              2) To implement both in 1.9.x and 2.0.x branches

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Moving to stable backlog, randomly assigning to Andrew and raising to critical to be included in next performance-focussed sprint @ HQ. Some notes: 1) Take a look to MDL-24695 (already assigned to Andrew). That shows the correct solution: one non-unique index by userid and fieldid. 2) To implement both in 1.9.x and 2.0.x branches Ciao
              Hide
              andyjdavis Andrew Davis added a comment -

              Ive committed a slightly modified version of the patch from MDL-24695 into my github account for Moodle 2.0.

              repo: git://github.com/andyjdavis/moodle.git
              branch: MDL-17201_index
              diff: https://github.com/andyjdavis/moodle/compare/master...MDL-17201_index

              Doing the 1.9 version next.

              Show
              andyjdavis Andrew Davis added a comment - Ive committed a slightly modified version of the patch from MDL-24695 into my github account for Moodle 2.0. repo: git://github.com/andyjdavis/moodle.git branch: MDL-17201 _index diff: https://github.com/andyjdavis/moodle/compare/master...MDL-17201_index Doing the 1.9 version next.
              Hide
              andyjdavis Andrew Davis added a comment -

              1.9 done

              repo: git://github.com/andyjdavis/moodle.git
              branch: MDL-17201_index_19
              diff: https://github.com/andyjdavis/moodle/compare/MOODLE_19_STABLE...MDL-17201_index_19

              Note when testing that, as this adds an index to a table, this involves your db being upgraded.

              Show
              andyjdavis Andrew Davis added a comment - 1.9 done repo: git://github.com/andyjdavis/moodle.git branch: MDL-17201 _index_19 diff: https://github.com/andyjdavis/moodle/compare/MOODLE_19_STABLE...MDL-17201_index_19 Note when testing that, as this adds an index to a table, this involves your db being upgraded.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Hi Andrew,
              Both look good and get my +1 on the peer-review.

              Cheers
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - Hi Andrew, Both look good and get my +1 on the peer-review. Cheers Sam
              Hide
              andyjdavis Andrew Davis added a comment -

              PULL-165 for 1.9

              PULL-164 for 2.0

              Show
              andyjdavis Andrew Davis added a comment - PULL-165 for 1.9 PULL-164 for 2.0
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Both PULL-164 and PULL-165 integrated and tests passing ok. Closing this a fixed. Thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Both PULL-164 and PULL-165 integrated and tests passing ok. Closing this a fixed. Thanks!

                People

                • Votes:
                  10 Vote for this issue
                  Watchers:
                  8 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    21/Feb/11