Answers.Body: <p>I don't know the original authors' intent but can find some background and share a theory:</p>
  
  <ol>
  <li><p><em>Background</em>: To figure out the origin of some code, it sometimes helps to spot-check the <a href="http://svn.civicrm.org:81/viewvc/civicrm/branches/" rel="nofollow">branches in the old Subversion repository</a>. This can require a bit of digging (to trace through incidental merges and cleanups in the history), but ultimately the <code>shuffle()</code> originated in <a href="http://svn.civicrm.org:81/viewvc/civicrm?view=revision&amp;revision=35243" rel="nofollow">r35243</a> and <a href="https://issues.civicrm.org/jira/browse/CRM-8460" rel="nofollow">CRM-8460</a>. Alas, there's no commentary in there about this detail. You could ping the original author.</p></li>
  <li><p><em>Theory</em>: Statistically, shuffling reduces the number of lock-acquisition attempts in one typical scenario. Suppose you're starting up a bunch of workers at the same time. (Maybe you're starting 5 workers, but you've set a limit of 10 workers.) Compare what happens with and without random polling order:</p>
  
  <ul>
  <li>(Without <code>shuffle()</code>) When worker #2 starts and tries its first attempt at lock-acquisition, there's a 100% chance of failure, so it will have to do a second lock-acquisition attempt. Similarly, when worker #3 starts and tries its first lock-acquisition, there's a 100% chance of failure.</li>
  <li>(With <code>shuffle()</code>) When worker #2 starts and tries its first lock-acquisition, there's 10% (1/10) chance of failure. Similarly, when worker #3 starts, there's a 20% chance.</li>
  </ul></li>
  </ol>
  
  <p>Now, even if we accept this theory, it might not amount to much. In a typical Civi deployment, you have a single server with maybe 1-10 workers, and locks are acquired infrequently. The total overhead isn't very high (esp. in comparison to the work being done). However, if one had thousands of workers, or if the communications channels were slow, or if you had to acquire the locks frequently (every second), then it would add up... so it wouldn't be surprising if some folks viewed it as a generally good practice to shuffle when polling.</p>
  
Answers.ContentLicense: CC BY-SA 3.0
Answers.CreationDate: 2015-12-01T20:41:36.070Z
Answers.LastActivityDate: 2015-12-02T18:06:33.560Z
Answers.LastEditDate: 2015-12-02T18:06:33.560Z
Answers.UserDisplayName: Tim Otten
Answers.Body: <p><s>There doesn't seem to be a good reason, so: <a href="https://github.com/civicrm/civicrm-core/pull/7338" rel="nofollow">https://github.com/civicrm/civicrm-core/pull/7338</a></s></p>
  
  <p>Tim's answer demonstrates that there is a good reason, so PR closed =]</p>
  
Answers.ContentLicense: CC BY-SA 3.0
Answers.CreationDate: 2015-11-30T09:16:38.920Z
Answers.LastActivityDate: 2015-12-01T21:54:21.553Z
Answers.LastEditDate: 2015-12-01T21:54:21.553Z
Answers.UserDisplayName: John
Body: <p><a href="https://github.com/civicrm/civicrm-core/blob/a2976e7c0c2515379e98ee9b5ebcf51acde4e540/CRM/Mailing/BAO/Mailing.php#L2867" rel="nofollow">https://github.com/civicrm/civicrm-core/blob/a2976e7c0c2515379e98ee9b5ebcf51acde4e540/CRM/Mailing/BAO/Mailing.php#L2867</a></p>
  
  <pre><code>  $lockArray = range(1, $mailerJobsMax);
    shuffle($lockArray);
  
    // check if we are using global locks
    foreach ($lockArray as $lockID) {
      $cronLock = Civi::lockManager()-&gt;acquire("worker.mailing.send.{$lockID}");
      if ($cronLock-&gt;isAcquired()) {
        $gotCronLock = TRUE;
        break;
      }
    }
  </code></pre>
  
  <p>Why do we randomise the order of the array using shuffle()? Is there a good reason for doing this?</p>
  
Comments.ContentLicense: CC BY-SA 3.0
Comments.CreationDate: 2015-11-26T15:37:59.517Z
Comments.Text: Good question. And I don't have the answer. May I ask why you are asking this question?
Comments.UserDisplayName: Jaap Jansma - CiviCooP
Comments.ContentLicense: CC BY-SA 3.0
Comments.CreationDate: 2015-11-26T15:49:19.357Z
Comments.Text: If it's there for a reason then great - I'd love to know why. If it's not then I'll get rid of it as part of some other tidying. I'm currently picking apart the Mailing process, trying to debug something, so anything that looks a bit 'weird' is throwing me off ;]
Comments.UserDisplayName: John
Comments.ContentLicense: CC BY-SA 3.0
Comments.CreationDate: 2015-12-01T20:28:51.630Z
Comments.Text: @JaapJansma when you write "good question" don't forget to also up-vote :)
Comments.UserDisplayName: Coleman
Tags: <civimail><php>
Title: Why does the mailer processQueue method 'shuffle' the array of possible locks?
ThreadText: # Why does the mailer processQueue method 'shuffle' the array of possible locks?
  
  (Asked by: username_0 on 2015-11-26)
  
  https://github.com/civicrm/civicrm-core/blob/a2976e7c0c2515379e98ee9b5ebcf51acde4e540/CRM/Mailing/BAO/Mailing.php#L2867
    $lockArray = range(1, $mailerJobsMax);
    shuffle($lockArray);
  
    // check if we are using global locks
    foreach ($lockArray as $lockID) {
      $cronLock = Civi::lockManager()->acquire("worker.mailing.send.{$lockID}");
      if ($cronLock->isAcquired()) {
        $gotCronLock = TRUE;
        break;
      }
    }
  
  Why do we randomise the order of the array using shuffle()? Is there a good reason for doing this?
  
  **Tags:** civimail, php
  
  > **Comments:**
  > **username_1**: Good question. And I don't have the answer. May I ask why you are asking this question?  
  > **username_0**: If it's there for a reason then great - I'd love to know why. If it's not then I'll get rid of it as part of some other tidying. I'm currently picking apart the Mailing process, trying to debug something, so anything that looks a bit 'weird' is throwing me off ;]  
  > **username_2**: @JaapJansma when you write "good question" don't forget to also up-vote :)  
  
  ---
  
  ## 2 answers
  
  ### Answer by username_3 on 2015-12-01. Score: 3 | Accepted answer
  
  I don't know the original authors' intent but can find some background and share a theory:
  
  1. *Background*: To figure out the origin of some code, it sometimes helps to spot-check the [branches in the old Subversion repository](http://svn.civicrm.org:81/viewvc/civicrm/branches/). This can require a bit of digging (to trace through incidental merges and cleanups in the history), but ultimately the `shuffle()` originated in [r35243](http://svn.civicrm.org:81/viewvc/civicrm?view=revision&revision=35243) and [CRM-8460](https://issues.civicrm.org/jira/browse/CRM-8460). Alas, there's no commentary in there about this detail. You could ping the original author.
  2. *Theory*: Statistically, shuffling reduces the number of lock-acquisition attempts in one typical scenario. Suppose you're starting up a bunch of workers at the same time. (Maybe you're starting 5 workers, but you've set a limit of 10 workers.) Compare what happens with and without random polling order:
  
     * (Without `shuffle()`) When worker #2 starts and tries its first attempt at lock-acquisition, there's a 100% chance of failure, so it will have to do a second lock-acquisition attempt. Similarly, when worker #3 starts and tries its first lock-acquisition, there's a 100% chance of failure.
     * (With `shuffle()`) When worker #2 starts and tries its first lock-acquisition, there's 10% (1/10) chance of failure. Similarly, when worker #3 starts, there's a 20% chance.
  
  Now, even if we accept this theory, it might not amount to much. In a typical Civi deployment, you have a single server with maybe 1-10 workers, and locks are acquired infrequently. The total overhead isn't very high (esp. in comparison to the work being done). However, if one had thousands of workers, or if the communications channels were slow, or if you had to acquire the locks frequently (every second), then it would add up... so it wouldn't be surprising if some folks viewed it as a generally good practice to shuffle when polling.
  
  
  ---
  
  ### Answer by username_0 on 2015-11-30. Score: 0
  
  ~~There doesn't seem to be a good reason, so: <https://github.com/civicrm/civicrm-core/pull/7338>~~
  
  Tim's answer demonstrates that there is a good reason, so PR closed =]
  
  
  ---
  
  