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

notify_login_failures() performance issue

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.4, 2.4.1, 2.5, 2.7.3, 2.8.1, 2.9
    • Fix Version/s: 2.7.4, 2.8.2
    • Component/s: Performance
    • Labels:
    • Testing Instructions:
      Hide
      1. Search for notifyloginfailures config and set it to "Admin user"
      2. Go to data base table mdl_config and search for setting with name "lastnotifyfailure". Delete the entry if found.
      3. In method \core\task\send_failed_login_notifications_task::execute() add the following code after the if ($CFG->notifyfailure ...) statement around line 61

         
                mtrace($CFG->lastnotifyfailure);
        

      4. Run "php admin/tool/task/cli/schedule_task.php --execute=\\core\\task
        send_failed_login_notifications_task"
      5. Make sure the time echoed is roughly (time() - 2592000)
      Show
      Search for notifyloginfailures config and set it to "Admin user" Go to data base table mdl_config and search for setting with name "lastnotifyfailure". Delete the entry if found. In method \core\task\send_failed_login_notifications_task::execute() add the following code after the if ($CFG->notifyfailure ...) statement around line 61   mtrace($CFG->lastnotifyfailure); Run "php admin/tool/task/cli/schedule_task.php --execute=\\core\\task send_failed_login_notifications_task" Make sure the time echoed is roughly (time() - 2592000)
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_27_STABLE, MOODLE_28_STABLE, MOODLE_29_STABLE
    • Fixed Branches:
      MOODLE_27_STABLE, MOODLE_28_STABLE
    • Pull Master Branch:
      MDL-37584-master
    • Sprint:
      Team B Sprint 1

      Description

      There is a possible performance issue with function notify_login_failures() the first time login failures are checked. If they were never checked previously (e.g. $CFG->notifyloginfailures was not set), $CFG->lastnotifyfailure will be empty. In such a case, the whole log will be checked as per this logic:

          if (empty($CFG->lastnotifyfailure)) {
              $CFG->lastnotifyfailure=0;
          }
      

      If the log is big, this potentially matches thousands of records. Even worse, if the function does not finish (e.g. because connection to DB will time out), $CFG->lastnotifyfailure will never be set to the current date. Hence the same situation will repeat with the next cron run.

      As a fix, I would suggest to never check more than (say) 1 last month.

        Attachments

          Issue Links

            Activity

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Jan/15