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 from Repository:
    • Pull Master Branch:
      MDL-37584-master
    • Pull Master Diff URL:
    • 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

          has been marked as being related by

          Improvement - An enhancement to an existing Moodle feature. MDL-42891 Reimplement access to log table in functions notify_login_failures() and count_login_failures()

          • Minor - Minor loss of function where workaround is possible
          • Closed

            Activity

              People

              Assignee:
              ankit_frenz Ankit Agarwal
              Reporter:
              tmuras Tomasz Muras
              Peer reviewer:
              Dan Poltawski
              Integrator:
              Eloy Lafuente (stronk7)
              Tester:
              Eloy Lafuente (stronk7)
              Participants:
              Component watchers:
              Matteo Scaramuccia, Andrew Nicols, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

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