Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-9559

Messenger Queue Batch Size configuration option is overridden

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.7-PL1
    • Fix Version/s: 3.0.8-RC1
    • Component/s: Other
    • Labels:
      None

      Description

      I have 4,000+ members and I wish to send a mass email. In checking for queuing (queueing?), I've seen people complain on the phpbb3 support forum that emails are not being throttled, even when they've set the batch size very low. Since my host limits emails to 1,000 per hour and fails any messages above that limit, I decided to have a looksee at the code before attempting to send the mass email.

      In includes/functions_messenger.php, $num_items seems to be used to control how many messages are sent in the current execution of the function. It appears that $num_items is set properly at first, but then overridden and set to the total number of messages in the queue if the number of messages in the queue is 2.5 times the batch size.

      It seems to me to be completely overriding the user's configuration defeating the purpose of the queue, and the configuration limit.

               if (!isset($data_ary['package_size']))
               {
                  $data_ary['package_size'] = 0;
               }
       
               $package_size = $data_ary['package_size'];
               $num_items = (!$package_size || sizeof($data_ary['data']) < $package_size) ? sizeof($data_ary['data']) : $package_size;
       
               // If the amount of emails to be sent is way more than package_size than we need to increase it to prevent backlogs...
               if (sizeof($data_ary['data']) > $package_size * 2.5)
               {
                  $num_items = sizeof($data_ary['data']);
               }
       
      .
      .
      .
      [later in the code]
      .
      .
      .
              for ($i = 0; $i < $num_items; $i++)
               {
                  // Make variables available...
                  extract(array_shift($this->queue_data[$object]['data']));
       
                  switch ($object)
                  {
                     case 'email':
                        $err_msg = '';
                        $to = (!$to) ? 'undisclosed-recipients:;' : $to;
       
                        if ($config['smtp_delivery'])
                        {
                           $result = smtpmail($addresses, mail_encode($subject), wordwrap(utf8_wordwrap($msg), 997, "\n", true), $err_msg, $headers);
      [code continues...]
      

      Some hosts have a very strict limit on number of addresses per batch. For instance, one host in particular has a limit of 4. This approach would not ever work for anyone on that host attempting to send more than 10 messages at a time.

      I believe the following code should be removed as its current operation appears to be incorrect.

               // If the amount of emails to be sent is way more than package_size than we need to increase it to prevent backlogs...
               if (sizeof($data_ary['data']) > $package_size * 2.5)
               {
                  $num_items = sizeof($data_ary['data']);
               }
      

        Issue Links

          Activity

          Hide
          schnorrer42 schnorrer42 added a comment -

          I wasn't really sure if this should be opened as a "Minor" or "Major" issue.

          Show
          schnorrer42 schnorrer42 added a comment - I wasn't really sure if this should be opened as a "Minor" or "Major" issue.
          Hide
          nickvergessen Joas Schilling added a comment -

          made code read/scrollable.

          Show
          nickvergessen Joas Schilling added a comment - made code read/scrollable.
          Hide
          A_Jelly_Doughnut A_Jelly_Doughnut added a comment -

          See useful discussion at PHPBB3-9586

          Show
          A_Jelly_Doughnut A_Jelly_Doughnut added a comment - See useful discussion at PHPBB3-9586
          Hide
          schnorrer42 schnorrer42 added a comment -

          I don't mean to be rude or anything, but I read the potential reasons behind the override, and I find it a little bit difficult to understand why a problem created by a few admins who set their limits too low should cause the functionality for everyone else to be lost?

          I probably missed or misunderstood something though. That happens more often as I've been aging, so it's entirely likely.

          Thanks for looking at this.

          Show
          schnorrer42 schnorrer42 added a comment - I don't mean to be rude or anything, but I read the potential reasons behind the override, and I find it a little bit difficult to understand why a problem created by a few admins who set their limits too low should cause the functionality for everyone else to be lost? I probably missed or misunderstood something though. That happens more often as I've been aging, so it's entirely likely. Thanks for looking at this.
          Hide
          A_Jelly_Doughnut A_Jelly_Doughnut added a comment -

          This fix does not resolve any "deep" queue problems, but it does get rid of the admin setting override.

          Show
          A_Jelly_Doughnut A_Jelly_Doughnut added a comment - This fix does not resolve any "deep" queue problems, but it does get rid of the admin setting override.

            People

            • Assignee:
              A_Jelly_Doughnut A_Jelly_Doughnut
              Reporter:
              schnorrer42 schnorrer42
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development