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

Regression in login performance with enrol_database

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide
      Before applying the patch:
      1. Generate a large database for external authentication. You can do so using this repo: https://github.com/andrewnicols/mdl_auth_db - I created 2500 users, and 2400 courses, with a max users / course of 1000 - you don't need this many. Perhaps go for 1000, 1000, 1000 at most.
      2. Ensure that enrol_database, and auth_db are configured correctly and have been run recently
      3. Run database plugin CLI to test sync_enrolments()
      4. Ensure that PERF_DB and friends are enabled
      5. Choose a user and find their ID
      6. Download https://gist.github.com/andrewnicols/75ef940135128eae2b22 to test.php and update user ID
      7. run test.php several times and note the run times + DB stats
      Apply the patch
      1. run test.php several times and note the run times + DB stats
      2. Compare:
        1. run times - they should be faster
        2. DB reads - they should be less than / equal
        3. DB writes - they should be roughly equal (there may be 1 or 2 different due to caching/sessions/etc)
      Show
      Before applying the patch: Generate a large database for external authentication. You can do so using this repo: https://github.com/andrewnicols/mdl_auth_db - I created 2500 users, and 2400 courses, with a max users / course of 1000 - you don't need this many. Perhaps go for 1000, 1000, 1000 at most. Ensure that enrol_database, and auth_db are configured correctly and have been run recently Run database plugin CLI to test sync_enrolments() Ensure that PERF_DB and friends are enabled Choose a user and find their ID Download https://gist.github.com/andrewnicols/75ef940135128eae2b22 to test.php and update user ID run test.php several times and note the run times + DB stats Apply the patch run test.php several times and note the run times + DB stats Compare: run times - they should be faster DB reads - they should be less than / equal DB writes - they should be roughly equal (there may be 1 or 2 different due to caching/sessions/etc)
    • Affected Branches:
      MOODLE_28_STABLE, MOODLE_29_STABLE, MOODLE_30_STABLE
    • Fixed Branches:
      MOODLE_29_STABLE, MOODLE_30_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      One of the changes introduced in MDL-28420 has caused a regression in the login performance for users. We've observed the time taken to login jump between 3~40x longer, depending on the user and the number of courses they're enrolled in.

      The root cause of this is a change in one of the queries in the enrol_database sync. The JOIN to the user_enrolment table has changed to a LEFT JOIN to it. Which is otherwise fine, except that the user_enrolment join no longer has a userid filter associated with it.

      Example without MDL-28420:

      select count(1) from (SELECT e.*, c.visible AS cvisible, ue.status AS ustatus
      FROM mdl_enrol e
      JOIN mdl_user_enrolments ue ON ue.enrolid = e.id
      JOIN mdl_course c ON c.id = e.courseid
      WHERE ue.userid = 92774 AND e.enrol = 'database') x;
      

      count
      -------
      43

      Example with MDL-28420:

      select count(1) from (SELECT e.*, c.visible AS cvisible, ue.status AS ustatus
      FROM mdl_enrol e
      JOIN mdl_course c ON c.id = e.courseid
      JOIN mdl_role_assignments ra ON ra.itemid = e.id
      LEFT JOIN mdl_user_enrolments ue ON ue.enrolid = e.id
      WHERE ra.userid = 92774 AND e.enrol = 'database') x;
      

      count
      --------
      126672

        Attachments

          Issue Links

            Activity

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Jan/16