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

notify_login_failures() performance issue

    XMLWordPrintable

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 2.3.4, 2.4.1, 2.5, 2.7.3, 2.8.1, 2.9
    • 2.7.4, 2.8.2
    • Performance
    • MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_27_STABLE, MOODLE_28_STABLE, MOODLE_29_STABLE
    • MOODLE_27_STABLE, MOODLE_28_STABLE
    • MDL-37584-master
    • Easy
    • 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)
    • 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

              ankit_frenz Ankit Agarwal
              tmuras Tomasz Muras
              Dan Poltawski Dan Poltawski
              Eloy Lafuente (stronk7) Eloy Lafuente (stronk7)
              Eloy Lafuente (stronk7) Eloy Lafuente (stronk7)
              Matteo Scaramuccia, David Woloszyn, Huong Nguyen, Jake Dallimore, Meirza, Michael Hawkins, Raquel Ortega, Safat Shahin, Stevani Andolo
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                12/Jan/15