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

there is a potential preformance trouble in upgrade_set_timeout() in that its calling get_config() too many times.


    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Cannot Reproduce
    • Affects Version/s: 2.0.2, 2.1
    • Fix Version/s: None
    • Component/s: Administration
    • Labels:
    • Affected Branches:


      in MDL-26580 Petr has described concern for performance problems with upgrade_set_timeout() as well as adding a no-timeout option for cli mode upgrade:

      (16:38:10) skodak@moodle.org: arrggh, I might have found a bug in my upgrade_set_timeout()
      (16:38:23) skodak@moodle.org: could you please review it for me?
      (16:38:27) apu: really?? sure
      (16:39:11) skodak@moodle.org: the question is - does it execute get_config() too often?
      (16:39:59) apu: !isset($CFG->upgraderunning) or $CFG->upgraderunning < time()
      (16:40:00) apu: hm
      (16:40:06) skodak@moodle.org: yes
      (16:40:53) skodak@moodle.org: if (!isset($CFG->upgraderunning) or $CFG->upgraderunning + XXX < time()) {
      (16:41:44) skodak@moodle.org: I suppose 10 could work there
      (16:42:11) apu: tricky, how much to use is how much u save
      (16:42:47) skodak@moodle.org: it would be critical if you use it in large loops like those you added
      (16:44:02) skodak@moodle.org: well, not really critical
      (16:44:22) apu: yes, it would help, what is the risk.. that between 0- 10 we skip get_config and assume running
      (16:44:44) skodak@moodle.org: no big deal, if anybody interrupts upgrade it may create problems anyway
      (16:45:16) apu: ahh.. yea, interrupt as in cli ctrl-c ?
      (16:45:37) skodak@moodle.org: hmmmmmmmmmmmmmm
      (16:45:53) skodak@moodle.org: I think can remove the timeouts completely in CLI mode
      (16:46:30) apu: yes, cli mode should be much more interactive (or it can be)
      (16:47:02) skodak@moodle.org: the problem is when you stop web loading the upgrade has to continue until the next step to get consistent data, on the other hand ppl using CLI do not ctrl-C when it runs
      (16:50:53) skodak@moodle.org: did you plan to fix some other upgrade issues next week?
      (16:51:35) apu: i don't have any, not even planned the next 'sprint' ... i can work on it still if there are any
      (16:52:33) skodak@moodle.org: the problem is that the MDL-26580 should be resolved by the PULL, but I think it might not be
      (16:53:11) apu: going with reports so far, it is, but there is no other trace given (from Luis)
      (16:53:26) skodak@moodle.org: did he test it?
      (16:53:32) apu: no
      (16:53:48) apu: no one did yet
      (16:53:59) skodak@moodle.org: that is not good
      (16:54:25) apu: yea.. i know.. i'll ask in tracker
      (16:54:57) apu: grr, MDL-17344 for MOODLE_20_STABLE keeps conflicting..
      (16:55:11) apu: espcially version.php
      (16:55:24) skodak@moodle.org: yeah, tricky
      (16:57:40) skodak@moodle.org: I am a bit scared to commit your patch without fixing the upgrade_set_timeout(), but at the same time I am not 100% sure the 10s is good there
      (16:58:15) skodak@moodle.org: also it would be great to get some testing from the reporter
      (16:58:43) apu: yea it would shall we get him to test the pull or test in tracker?
      (16:58:56) skodak@moodle.org: maybe we could add the 10s there and let him use your branch for testing

        Gliffy Diagrams




              • Votes:
                1 Vote for this issue
                2 Start watching this issue


                • Created: