Moodle
  1. Moodle
  2. MDL-35109

Moodle sites are fetching available updates too often

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3.2
    • Component/s: Administration
    • Labels:
    • Testing Instructions:
      Hide

      Make sure that your site does not produce more than one cron-based request for available updates info in 24 hours.

      (if it sounds crazy, leave the testing to Eloy. He knows what to do

      Show
      Make sure that your site does not produce more than one cron-based request for available updates info in 24 hours. (if it sounds crazy, leave the testing to Eloy. He knows what to do
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-35109-available-updates
    • Rank:
      43735

      Description

      Moodle sites are supposed to regularly check for available updates via cron once per day every night. However, Eloy noticed that we have much more requests per site than expected.

        Issue Links

          Activity

          Hide
          David Mudrak added a comment -

          I'm on it.

          Show
          David Mudrak added a comment - I'm on it.
          Hide
          David Mudrak added a comment -

          So, it turned out that the problem was in available_update_checker::cron_has_fresh_fetch() method. It considered a fetch as a fresh only if it was not older than one hour. The correct value is 24 hours to respect the original design:

          • if the recently fetched data is older than 48 hours, it is considered as outdated and the new fetch is executed
          • else, if the recently fetched data is younger than 24 hours, it is considered as fresh enough and no fetch is executed
          • else, if the current time is somewhere between 01:00 AM and 06:00 AM (the certain offset is randomly generated for each site), the fetch is executed.
          Show
          David Mudrak added a comment - So, it turned out that the problem was in available_update_checker::cron_has_fresh_fetch() method. It considered a fetch as a fresh only if it was not older than one hour. The correct value is 24 hours to respect the original design: if the recently fetched data is older than 48 hours, it is considered as outdated and the new fetch is executed else, if the recently fetched data is younger than 24 hours, it is considered as fresh enough and no fetch is executed else, if the current time is somewhere between 01:00 AM and 06:00 AM (the certain offset is randomly generated for each site), the fetch is executed.
          Hide
          David Mudrak added a comment -

          Submitting a patchset for integration.

          • Added/improved unit tests to illustrate the problem
          • Fixed the bug.
          Show
          David Mudrak added a comment - Submitting a patchset for integration. Added/improved unit tests to illustrate the problem Fixed the bug.
          Hide
          Michael de Raadt added a comment -

          If you have created unit tests for this behaviour, we probably don't need a functional test.

          Although Eloy can fix anything.

          Show
          Michael de Raadt added a comment - If you have created unit tests for this behaviour, we probably don't need a functional test. Although Eloy can fix anything.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Sam Hemelryk added a comment -

          Thanks David, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks David, this has been integrated now
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yay, I (and download.moodle.org) will notice this, for sure, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Yay, I (and download.moodle.org) will notice this, for sure, thanks!
          Hide
          Michael de Raadt added a comment -

          In the latest integration the following error was shown (in PostgreSQL). It could have been because I had not run a plugins update in this instance before. I can't be sure that I hadn't as when I went to the Plugins management page, it showed the time of the failed attempt from cron.

          Outdated or missing info about available updates, forced fetching ... !!! Coding error detected, it must be fixed by a programmer: PHP catchable fatal error !!!
          !! Argument 1 passed to available_update_checker::compare_responses() must be an array, null given, called in D:\xampp\htdocs\moodle_master_test_postgreSQL\lib\pluginlib.php on line 1152 and defined
          Error code: codingerror !!
          !! Stack trace: * line 397 of \lib\setuplib.php: coding_exception thrown
          * line 901 of \lib\pluginlib.php: call to default_error_handler()
          * line 1152 of \lib\pluginlib.php: call to available_update_checker->compare_responses()
          * line 775 of \lib\pluginlib.php: call to available_update_checker->cron_execute()
          * line 394 of \lib\cronlib.php: call to available_update_checker->cron()
          * line 88 of \admin\cron.php: call to cron_run()
           !!
          
          Show
          Michael de Raadt added a comment - In the latest integration the following error was shown (in PostgreSQL). It could have been because I had not run a plugins update in this instance before. I can't be sure that I hadn't as when I went to the Plugins management page, it showed the time of the failed attempt from cron. Outdated or missing info about available updates, forced fetching ... !!! Coding error detected, it must be fixed by a programmer: PHP catchable fatal error !!! !! Argument 1 passed to available_update_checker::compare_responses() must be an array, null given, called in D:\xampp\htdocs\moodle_master_test_postgreSQL\lib\pluginlib.php on line 1152 and defined Error code: codingerror !! !! Stack trace: * line 397 of \lib\setuplib.php: coding_exception thrown * line 901 of \lib\pluginlib.php: call to default_error_handler() * line 1152 of \lib\pluginlib.php: call to available_update_checker->compare_responses() * line 775 of \lib\pluginlib.php: call to available_update_checker->cron_execute() * line 394 of \lib\cronlib.php: call to available_update_checker->cron() * line 88 of \admin\cron.php: call to cron_run() !!
          Hide
          Michael de Raadt added a comment -

          After I ran the cron manually, this error did not appear again in cron, but that could just have been because the wait time had not elapsed.

          Show
          Michael de Raadt added a comment - After I ran the cron manually, this error did not appear again in cron, but that could just have been because the wait time had not elapsed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just sent this to David some hours ago over Jabber:

          Hi David, just guessing...

          ...isn't that error the one you closed as dupe the other day assuming it was fixed time ago? (MDL-32338, the original and MDL-34140, the dupe).

          It seems the error is somehow persistent.

          But I don't think that should stop this because we were getting way too many requests in download.moodle.org. So I'm going to pass this (unit tests pass and some cron executions of 2 known servers say updates are not being requested so often).

          So passing... the only remaining point is the error commented by Michael that may need some investigation.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Just sent this to David some hours ago over Jabber: Hi David, just guessing... ...isn't that error the one you closed as dupe the other day assuming it was fixed time ago? ( MDL-32338 , the original and MDL-34140 , the dupe). It seems the error is somehow persistent. But I don't think that should stop this because we were getting way too many requests in download.moodle.org. So I'm going to pass this (unit tests pass and some cron executions of 2 known servers say updates are not being requested so often). So passing... the only remaining point is the error commented by Michael that may need some investigation. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many thanks for the hard work.

          These changes have been spread upstream and are already available in the git and cvs repositories.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work. These changes have been spread upstream and are already available in the git and cvs repositories. Ciao
          Hide
          David Mudrak added a comment -

          Thanks Michael for reporting the error. I was able to reproduce it and created a new issue MDL-35344.

          Show
          David Mudrak added a comment - Thanks Michael for reporting the error. I was able to reproduce it and created a new issue MDL-35344 .

            People

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

              Dates

              • Created:
                Updated:
                Resolved: