Always Claim the State Before Sending Anything Out

I ran into a case where a bot replied to a customer three times in the same minute. On my first pass through the code, I could not find the problem because the guard looked solid — read the state first, and if it is not queued, return. There seemed to be no gap. But once I looked at the queue logs, I understood what had happened.

The problem was not an incorrectly written guard. It was the time between "already sent" and "result written to the database", when the system has no record of what has already happened. This article covers the full set of duplicate prevention measures I put together after that incident.

The destination system names, table names, and numbers in this article are examples. The structure and code are real.

Reading the State Before Sending Is Not Enough

The code people naturally write looks like this, and it is wrong in a way that is hard to spot just by reading it.

// ผิด — ช่องโหว่อยู่ระหว่างบรรทัดที่ 5 กับ 6
public function handle(): void
{
    $row = DB::table('outbox')->find($this->id);
    if ($row->status !== 'queued') {
        return;                                   // guard ที่ดูแน่นหนา
    }
    $res = $this->gateway->send($row->target, $row->body);   // <- ยิงจริงตรงนี้
    DB::table('outbox')->where('id', $this->id)
      ->update(['status' => 'sent', 'external_id' => $res['id']]);
}

If the process gets killed, the machine reboots, or the worker hits a timeout between those two lines, the item really has been sent, but the database still says queued. When the queue releases the job for another attempt, the guard on the fourth line lets it straight through because the state is still queued, exactly as it expects.

Claim Before Sending, Then Check affected

The fix is not to make the guard stricter. It is to move the decision into the UPDATE statement itself, letting the database decide who gets the right to proceed instead of having the code decide for itself.

-- จองก่อนยิง: เปลี่ยนสถานะได้ = ได้สิทธิ์ยิง เปลี่ยนไม่ได้ = คนอื่นเอาไปแล้ว
UPDATE outbox
   SET status = 'sending', claimed_by = ?, claimed_at = NOW()
 WHERE id = ? AND status = 'queued';

public function handle(): void
{
    $claimed = DB::table('outbox')
        ->where('id', $this->id)->where('status', 'queued')
        ->update(['status' => 'sending', 'claimed_by' => $this->workerTag(), 'claimed_at' => now()]);

    if ($claimed !== 1) {
        Log::info('outbox.skip', ['id' => $this->id]);   // มีคนชิงไปแล้ว จบแค่นี้ ไม่ใช่ error
        return;
    }

    $res = $this->gateway->send(/* ... */);
    // ...เขียนผลกลับในหัวข้อถัดไป
}

The difference is the number in $claimed. It is the number of rows the statement actually changed, and the database guarantees that only one caller gets 1. Even if ten callers arrive at the same time, all the others get 0 and must stop immediately.

What I want to stress is that sending must be an actual state in the table, not a variable in memory. If the worker dies at this point, it leaves behind a row marked sending, which visibly shows that something stopped halfway through. I would rather have someone manually check whether the item reached its destination than let the system guess and send it again.

Writing the Result Back Needs a Guard Too

People miss this even more often than the first point. Once the claim succeeds, they tend to write the result back directly, immediately opening up another gap.

// ผิด — ทับสถานะโดยไม่ดูว่าระหว่างนั้นมีใครมาเปลี่ยนอะไรไปหรือเปล่า
DB::table('outbox')->where('id', $id)->update(['status' => 'sent']);

// ถูก — ยอมรับผลเฉพาะเมื่อแถวยังเป็นของเราอยู่
$done = DB::table('outbox')
    ->where('id', $id)->where('status', 'sending')->where('claimed_by', $me)
    ->update(['status' => 'sent', 'external_id' => $res['id'], 'sent_at' => now()]);

if ($done !== 1) {
    // คนกดยกเลิกระหว่างทาง ของขึ้นไปแล้วแต่ระบบบอกว่ายกเลิก -> ต้องตามลบ ไม่ใช่เงียบ
    Log::warning('outbox.cancelled_after_send', ['id' => $id, 'external_id' => $res['id']]);
    $this->gateway->delete($res['id']);
}

The scenario that makes this matter is someone clicking cancel in the web interface just as the worker is sending. If the write overwrites the state unconditionally, the system ends up saying "cancelled" even though the item really was published. Then the row disappears from the screen, and nobody can follow up and delete it anymore.

Cancellation Must Delete the Actual Item at the Destination

Following on from the previous point, cancelling in my own system is not a real cancellation if the item has already gone out. Deleting the row in my table is deleting the evidence, not withdrawing the item.

public function cancel(int $id): string
{
    $row = DB::table('outbox')->find($id);

    if ($row->status === 'queued') {
        $n = DB::table('outbox')->where('id', $id)->where('status', 'queued')
               ->update(['status' => 'cancelled']);
        return $n === 1 ? 'ยกเลิกแล้ว' : 'ยกเลิกไม่ทัน งานเริ่มส่งไปแล้ว';
    }

    if (in_array($row->status, ['sending', 'sent'], true) && $row->external_id) {
        $ok = $this->gateway->delete($row->external_id);
        if (! $ok) {
            // ลบที่ปลายทางไม่สำเร็จ ห้ามบอกว่ายกเลิกแล้วเด็ดขาด
            return 'ลบที่ปลายทางไม่สำเร็จ ของยังอยู่ ต้องตามเก็บเอง';
        }
        DB::table('outbox')->where('id', $id)->update(['status' => 'cancelled']);
        return 'ถอนออกจากปลายทางแล้ว';
    }

    return 'สถานะไม่รองรับการยกเลิก';
}

retry_after Must Exceed the Longest Job's Runtime

Another gap has nothing to do with the application code but can cause duplicate sends just the same: the queue connection's retry_after setting. This is how long the queue waits before deciding "this job is probably dead" and making it available for someone else to run again.

// config/queue.php
'connections' => [
    'redis' => [
        'driver'      => 'redis',
        'queue'       => 'default',
        'retry_after' => 900,     // ต้องมากกว่างานที่ยาวที่สุดเสมอ
        'block_for'   => null,
    ],
],

# worker ต้องตั้ง timeout ให้น้อยกว่า retry_after เสมอ ไม่ใช่มากกว่า
php artisan queue:work redis --timeout=600 --tries=3 --backoff=30

If retry_after=90 and a job actually takes two minutes, at second 90 the queue releases that same job for another worker to run while the first one is still working. The same job then runs in two places at once, which, for an outward job, means a duplicate send outright. Worse, this only happens with unusually slow jobs, meaning it shows up precisely when the system is already having problems.

These settings apply at the connection level and cannot be set per job. If the system has one very long job and all the others are short, my approach is to give the long job its own connection and set a long retry_after only for that connection, rather than increasing it for the entire system to accommodate the single slowest job. Doing that would also leave every genuinely dead job hanging longer than necessary.

Cache::lock Must Outlast the Job

If a lock provides another layer of protection against overlapping jobs, the rule is exactly the same — its lifetime must exceed the job's actual runtime, not the time I think it should take.

// lock 10 นาที ต้องคู่กับงานที่ไม่มีทางเกิน 10 นาที ไม่งั้นเปิดช่องรันซ้อนเอง
$lock = Cache::lock("outbox:{$id}", 600);

if (! $lock->get()) {
    return;                       // มีคนถืออยู่ ปล่อยผ่าน
}
try {
    $this->send($id);
} finally {
    $lock->release();             // ปล่อยใน finally เสมอ
}

The mistake people make is setting the lock to 60 seconds because "it normally finishes in 5 seconds." Then, on a day when the destination is slow, the job takes 90 seconds, the lock expires at second 60, a second worker gets in, and both send at once. My own rule is that the lock must always outlast the process timeout. Allowing extra time only costs some waiting, which is better than setting it too short and leaving a gap.

failed() Must Never Overwrite a Successful State

This last point is one I have got wrong myself. A job's failed() handler looks like the right place to record a failure, but it gets called at a time I cannot control.

// ผิด — งานที่เสร็จไปแล้วรอบแรก อาจโดนเขียนทับเป็น failed จากรอบที่สอง
public function failed(\Throwable $e): void
{
    DB::table('outbox')->where('id', $this->id)->update(['status' => 'failed']);
}

// ถูก — ยอมรับเฉพาะแถวที่ยังไม่สำเร็จเท่านั้น
public function failed(\Throwable $e): void
{
    DB::table('outbox')->where('id', $this->id)
      ->whereIn('status', ['queued', 'sending'])
      ->update(['status' => 'failed', 'error' => class_basename($e)]);
}

The scenario is a long job that the queue releases again because of retry_after — the first attempt succeeds a little late, the second fails, and the second attempt's failed() overwrites the successful state already written by the first. The system then reports a failure even though the item has already reached the customer, and someone clicks send again based on that report.

Checklist Before Deploying an Outward Job

I keep a short checklist to go through whenever I add this type of job. It takes less than ten minutes but prevents almost all the cases that cannot be cleaned up afterward.

1. Claim the state with a single UPDATE before sending, then verify that affected really equals 1. If it is 0, stop instead of continuing.
2. Writing the result back must require that the row still belongs to this worker. If it does not, something has intervened; handle it instead of staying silent.
3. Cancellation must delete the actual item at the destination first. If deletion fails, do not report it as cancelled.
4. Compare retry_after with the actual runtime of the longest job, not the average.
5. The lock lifetime must exceed the timeout of the process holding it.
6. failed() must include a condition that prevents overwriting a successful state.
7. Test by actually killing a worker midway through at least once, then check which state the row is left in.

The last point is the one people skip most often, and it is the most valuable. I submit one test job, wait for it to start sending, then kill -9 the worker. After that, I check the state left in the table, restart the worker, and see whether it makes the right decision.

# ทดสอบเส้นทางที่พังจริง — ทำกับปลายทางทดสอบเท่านั้น ห้ามยิงของจริง
php artisan tinker --execute="\App\Jobs\SendOutbox::dispatch(\$id);"
sleep 2
pkill -9 -f 'queue:work'
mysql appdb -e "SELECT id, status, claimed_by, external_id FROM outbox WHERE id = $ID\G"
systemctl start app-worker
sleep 10
mysql appdb -e "SELECT id, status, attempts, external_id FROM outbox WHERE id = $ID\G"

If the row is left at sending and someone has to check it manually, I consider that a pass. That is the intended behavior, not a failure. What must never happen is two successful completions or two external_id values from a single job.

If your constraints differ, adjust this as needed. But the one rule I will not bend is never send anything out before the system has recorded that it is sending. Get that order wrong, and everything else becomes cleanup. If I have left anything unexplained or made a typo, I will come back and fix it later.