Details

    • Type: Bug Bug
    • Status: Development in progress
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.9.9, 2.3.1
    • Fix Version/s: None
    • Component/s: Forum
    • Labels:
    • Environment:
      1.9.9+ (Build: 20100804), RedHat
    • Database:
      Any
    • Testing Instructions:
      Hide

      Prerequisites: You must a moodle with email sending enabled. It is useful to use $CFG->divertallemailto = '';

      1. Create a new course
      2. Enrol 10 users to the course
      3. Create a new standard forum, forced subscription
      4. Go to Forum > Settings > Show/Edit current subscribers
      5. VERIFY: that the 10 users are subscribed
      6. Change the settings to forum subscription to optional
      7. Go to Forum > Settings > Show/Edit current subscribers
      8. VERFIY: no users are subscribed
      9. Turn editing on
      10. Subscribe one of the users
      11. Turn editing off
      12. VERIFY: the user you enrolled is subscribed
      13. Subscribe another two users.
      14. Add a new post to the forum, click the 'mailnow' tickbox.
      15. Run cron manually.
      16. VERIFY: that the 3 users you subscribed received the email
      Show
      Prerequisites: You must a moodle with email sending enabled. It is useful to use $CFG->divertallemailto = ''; Create a new course Enrol 10 users to the course Create a new standard forum, forced subscription Go to Forum > Settings > Show/Edit current subscribers VERIFY: that the 10 users are subscribed Change the settings to forum subscription to optional Go to Forum > Settings > Show/Edit current subscribers VERFIY: no users are subscribed Turn editing on Subscribe one of the users Turn editing off VERIFY: the user you enrolled is subscribed Subscribe another two users. Add a new post to the forum, click the 'mailnow' tickbox. Run cron manually. VERIFY: that the 3 users you subscribed received the email
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
    • Rank:
      2686

      Description

      We have a suggestion to fix this issue by limiting the get_record parameters cached while processing the digest items and raising the forum cron to use 256Mb.

      Processing email digest for user 4366... PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 104 bytes) in /git/moodle/lib/weblib.php on line 1728

        Issue Links

          Activity

          Hide
          Tim Lock added a comment -

          This patch should reduce the data required from the user table to keep the memory usage down. Testing need.

          Show
          Tim Lock added a comment - This patch should reduce the data required from the user table to keep the memory usage down. Testing need.
          Hide
          Dan Poltawski added a comment -

          The originally reported issue in 1.9 has long since changed. But I'm going to use this issue to record my investigations into a current issue in 2.3.

          I just setup a test rig with 600,000 users enrolled and got:

          Processing module function forum_cron ...
          Fatal error: Allowed memory size of 536870912 bytes exhausted (tried to allocate 84 bytes) in /Users/danp/git/integration/lib/dml/pgsql_native_moodle_database.php on line 727
          
          Show
          Dan Poltawski added a comment - The originally reported issue in 1.9 has long since changed. But I'm going to use this issue to record my investigations into a current issue in 2.3. I just setup a test rig with 600,000 users enrolled and got: Processing module function forum_cron ... Fatal error: Allowed memory size of 536870912 bytes exhausted (tried to allocate 84 bytes) in /Users/danp/git/integration/lib/dml/pgsql_native_moodle_database.php on line 727
          Hide
          Dan Poltawski added a comment -

          Hmm ok, this is because of forum_subscribed_users/forum_get_potential_subscribers returning all the users at once, seems unlikely to be able to reachitect that right now.

          Show
          Dan Poltawski added a comment - Hmm ok, this is because of forum_subscribed_users/forum_get_potential_subscribers returning all the users at once, seems unlikely to be able to reachitect that right now.
          Hide
          Dan Poltawski added a comment -

          Running through the rest of the code, I think this subscribed check is one place where memory is growing completely unbounded and is fixable. I didn't have enough memory on my machine to allow this cron to run with 600K users. After converting to recordset it peaks at about 600MB (still not great, but acceptable for cron).

          Show
          Dan Poltawski added a comment - Running through the rest of the code, I think this subscribed check is one place where memory is growing completely unbounded and is fixable. I didn't have enough memory on my machine to allow this cron to run with 600K users. After converting to recordset it peaks at about 600MB (still not great, but acceptable for cron).
          Hide
          Dan Poltawski added a comment -

          We seem to be enforcing sorting on forum_get_potential_subscribers().

          This makes a massive difference on a big table like user, for my 600K users, the query takes 32s to run, 5 without sorting.

          Show
          Dan Poltawski added a comment - We seem to be enforcing sorting on forum_get_potential_subscribers(). This makes a massive difference on a big table like user, for my 600K users, the query takes 32s to run, 5 without sorting.
          Hide
          Dan Poltawski added a comment -

          OK, sending this fro peer review.

          I have:

          • Introduced new recordset functions to replace the existing get_records based functions. As noted above this allows a forum with over 600,000 subscribers to run peaking at about 700MB Ram, rather than >3GB
          • Converted the old functions to use the new functionality, this is a bit weird because I have to pretend the recordset is a get_records call. I manually index the array on userid - this is because thats what the existing functions actually depended upon.
          • Added unit tests for the old/new functions.

          Note that I haven't converted the subscription interface to use these recorset calls, this is because there are architectural problems which stop it working anyway (putting 600,000 users in one big table). So this is a half way house, but it means that the subscriptions will send out via email.

          Show
          Dan Poltawski added a comment - OK, sending this fro peer review. I have: Introduced new recordset functions to replace the existing get_records based functions. As noted above this allows a forum with over 600,000 subscribers to run peaking at about 700MB Ram, rather than >3GB Converted the old functions to use the new functionality, this is a bit weird because I have to pretend the recordset is a get_records call. I manually index the array on userid - this is because thats what the existing functions actually depended upon. Added unit tests for the old/new functions. Note that I haven't converted the subscription interface to use these recorset calls, this is because there are architectural problems which stop it working anyway (putting 600,000 users in one big table). So this is a half way house, but it means that the subscriptions will send out via email.
          Hide
          Dan Poltawski added a comment -

          Petr, it'd be great if you are able to review this.

          Show
          Dan Poltawski added a comment - Petr, it'd be great if you are able to review this.
          Hide
          Petr Škoda added a comment -

          1/ in function forum_get_potential_subscribers() and forum_subscribed_users() there is a missing $rs->close() when the resultset is empty
          2/ seems you have removed "ORDER BY u.email ASC" from forum_subscribed_users(), why? I would expect this to use $sort parameter.

          Show
          Petr Škoda added a comment - 1/ in function forum_get_potential_subscribers() and forum_subscribed_users() there is a missing $rs->close() when the resultset is empty 2/ seems you have removed "ORDER BY u.email ASC" from forum_subscribed_users(), why? I would expect this to use $sort parameter.
          Hide
          Dan Poltawski added a comment -

          Thanks for looking.

          Regarding 2/ Doing the sort makes a massive difference on big user tables (32s to run on my big site, 5s without sorting) and I couldn't spot why sorting by email was necessary.

          Show
          Dan Poltawski added a comment - Thanks for looking. Regarding 2/ Doing the sort makes a massive difference on big user tables (32s to run on my big site, 5s without sorting) and I couldn't spot why sorting by email was necessary.
          Hide
          Petr Škoda added a comment -

          If I remember it correctly Martin wanted to have some predictable mailing order when diagnosing/replicating problems. I do not mind removing it much, but the API should imo allow it: $sort="" would not make it slower and the old function could be 100% BC

          Show
          Petr Škoda added a comment - If I remember it correctly Martin wanted to have some predictable mailing order when diagnosing/replicating problems. I do not mind removing it much, but the API should imo allow it: $sort="" would not make it slower and the old function could be 100% BC
          Hide
          Dan Poltawski added a comment - - edited

          The following happened after a long time of running digests, seemingly the memory is creeping up. Trying to investigate.

          Processing user 121497
          Processing user 63685
          Processing user 387019
          Processing user 480985
          Processing user 335409
          PHP Fatal error:  Allowed memory size of 2147483648 bytes exhausted (tried to allocate 5461 bytes) in /Users/danp/git/integration/lib/dml/moodle_database.php on line 826
          
          Fatal error: Allowed memory size of 2147483648 bytes exhausted (tried to allocate 5461 bytes) in /Users/danp/git/integration/lib/dml/moodle_database.php on line 826
          
          Show
          Dan Poltawski added a comment - - edited The following happened after a long time of running digests, seemingly the memory is creeping up. Trying to investigate. Processing user 121497 Processing user 63685 Processing user 387019 Processing user 480985 Processing user 335409 PHP Fatal error: Allowed memory size of 2147483648 bytes exhausted (tried to allocate 5461 bytes) in /Users/danp/git/integration/lib/dml/moodle_database.php on line 826 Fatal error: Allowed memory size of 2147483648 bytes exhausted (tried to allocate 5461 bytes) in /Users/danp/git/integration/lib/dml/moodle_database.php on line 826
          Hide
          Dan Poltawski added a comment -

          This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.
          For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

          Show
          Dan Poltawski added a comment - This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

            People

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

              Dates

              • Created:
                Updated: