CiviCRM Stack Exchange is a question and answer site for administrators and users of the CiviCRM Constituent Relationship Management software. It only takes a minute to sign up.
Sign up to join this community
$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?
I don't know the original authors' intent but can find some background and share a theory:
Background: To figure out the origin of some code, it sometimes helps to spot-check the branches in the old Subversion repository. This can require a bit of digging (to trace through incidental merges and cleanups in the history), but ultimately the shuffle() originated in r35243 and CRM-8460. Alas, there's no commentary in there about this detail. You could ping the original author.
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:
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.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.
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 =]
