Last Updated: 2024-11-23

I had the following code for sending out emails in a background queue. Unfortunately, due to a temporary network outage causing http requests to our geocoder to fail, the job remained locked and could not be retried.

Here was the code:

 <?php

if ($this->reverseAuctionRequest->isLocked()) {
    Log::info('Ignoring what seems like a duplicate ReverseAuctionRequest job');
    return;
}

// This helps us prevent the situation where we process
// something multiple times by access and send out too many emails.
$this->reverseAuctionRequest->lockToPreventSimultaneousProcessing();
$advisors = $this->chooseAdvisors(10);
sendEmails($advisors)
$this->reverseAuctionRequest->releaseLock();
...

private function chooseAdvisors($numberToContact) : iterable
{
    $geocodeResults = Http::getGeocood($address);
    ... // Happy path not depicted
    if (! is_null($geocodeResults["latitude"])) {
      throw new \Exception("Could not calculate geocords.");
    }
  }

The issue was that I did not release the lock when an exception with the geocoder occurred. One solution would be to release in a catch statement, no matter what. Another would be to do the idempotent querying first to grab the advisors first (triggering the possible geocoding exception earlier) and then locking at last possible moment.

<? php
    $advisors = $this->chooseAdvisors(10);
    // moved down until after the non-mutating querying
    $this->reverseAuctionRequest->beginProcessing();
    sendEmails($advisors)
    $this->reverseAuctionRequest->finishProcessing();

Lessons

• Only lock a record at last minute and for as short a time as possible.
• Release locks during exceptions