งานที่ยิงออกนอกระบบ ต้องจองสถานะก่อนยิงเสมอ

พอดีเจอเคสที่บอทตอบข้อความหาลูกค้าซ้ำสามรอบในนาทีเดียวกัน ตอนไล่ดูโค้ดครั้งแรกผมก็หาไม่เจอ เพราะมันเขียน guard ไว้ดูดีมาก — อ่านสถานะก่อน ถ้าไม่ใช่ queued ก็ return ทิ้ง ดูไม่มีช่องเลย แต่พอไปดู log ของคิวถึงเข้าใจว่าเกิดอะไรขึ้น

ปัญหาไม่ได้อยู่ที่ guard เขียนผิด ปัญหาอยู่ที่ช่วงเวลาระหว่าง "ยิงออกไปแล้ว" กับ "เขียนผลลงฐานข้อมูล" ซึ่งเป็นช่วงที่ระบบไม่มีบันทึกอะไรเลยว่าเกิดอะไรขึ้นไปแล้วบ้าง บทความนี้คือชุดกันซ้ำทั้งชุดที่ผมสรุปไว้หลังเคสนั้น

ชื่อระบบปลายทาง ชื่อตาราง และตัวเลขในบทความเป็นค่าตัวอย่าง โครงและโค้ดเป็นของจริง

อ่านสถานะแล้วค่อยยิง ยังไม่พอ

โค้ดที่คนเขียนกันโดยธรรมชาติหน้าตาแบบนี้ และมันผิดในแบบที่มองไม่เห็นตอนอ่าน

// ผิด — ช่องโหว่อยู่ระหว่างบรรทัดที่ 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']]);
}

ถ้า process โดนฆ่า, เครื่องรีบูต, หรือ worker ชน timeout ระหว่างสองบรรทัดนั้น สิ่งที่เกิดคือของถูกส่งออกไปแล้วจริง แต่ในฐานข้อมูลยังเป็น queued อยู่ พอคิวเด้งงานกลับมาลองใหม่ guard บรรทัดที่สี่จะปล่อยผ่านสบาย ๆ เพราะสถานะยังเป็น queued ตามที่มันคาดหวังพอดี

จองก่อนยิง แล้วเช็ก affected

วิธีแก้ไม่ได้อยู่ที่การเขียน guard ให้แน่นขึ้น แต่อยู่ที่การย้ายการตัดสินใจไปไว้ในคำสั่ง UPDATE เอง ให้ฐานข้อมูลเป็นคนบอกว่าใครได้สิทธิ์ ไม่ใช่ให้โค้ดเราเป็นคนคิดเอง

-- จองก่อนยิง: เปลี่ยนสถานะได้ = ได้สิทธิ์ยิง เปลี่ยนไม่ได้ = คนอื่นเอาไปแล้ว
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(/* ... */);
    // ...เขียนผลกลับในหัวข้อถัดไป
}

ความต่างอยู่ที่ตัวเลข $claimed ครับ มันคือจำนวนแถวที่คำสั่งนั้นแก้ได้จริง ซึ่งฐานข้อมูลรับประกันให้ว่ามีคนเดียวที่จะได้ค่า 1 ต่อให้ยิงเข้ามาพร้อมกันสิบตัว ที่เหลือได้ 0 หมดและต้องหยุดทันที

ที่ผมอยากเน้นคือ สถานะ sending ต้องเป็นสถานะจริงในตาราง ไม่ใช่ตัวแปรในหน่วยความจำ เพราะถ้า worker ตายตอนนี้ สิ่งที่ค้างอยู่คือแถวที่เขียนว่า sending ซึ่งอ่านได้ด้วยตาว่ามีบางอย่างค้างกลางทาง ผมยอมให้คนไปเช็กเองว่าของถึงปลายทางหรือยัง ดีกว่าปล่อยให้ระบบเดาแล้วยิงซ้ำ

เขียนผลกลับก็ต้องมี guard เหมือนกัน

ข้อนี้คนพลาดกันเยอะกว่าข้อแรกอีก พอจองสำเร็จแล้วก็มักจะเขียนผลกลับแบบตรงไปตรงมา ซึ่งเปิดช่องใหม่ให้ทันที

// ผิด — ทับสถานะโดยไม่ดูว่าระหว่างนั้นมีใครมาเปลี่ยนอะไรไปหรือเปล่า
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']);
}

สถานการณ์ที่ทำให้ข้อนี้มีความหมายคือ คนกดยกเลิกในหน้าเว็บระหว่างที่ worker กำลังยิงอยู่พอดี ถ้าเราเขียนทับแบบไม่มีเงื่อนไข ระบบจะกลายเป็นบอกว่า "ยกเลิกแล้ว" ทั้งที่ของขึ้นไปจริง แล้วแถวนั้นก็หลุดออกจากหน้าจอไปเลย ไม่มีใครตามไปลบได้อีก

ยกเลิกต้องลบของจริงที่ปลายทาง

ต่อจากข้อที่แล้ว การยกเลิกในระบบเราไม่ใช่การยกเลิกจริง ถ้าของออกไปแล้ว การลบแถวในตารางเราคือการลบหลักฐาน ไม่ใช่การถอนของ

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 ต้องมากกว่างานที่ยาวที่สุด

อีกช่องหนึ่งที่ไม่เกี่ยวกับโค้ดของเราเลย แต่ทำให้ยิงซ้ำได้เหมือนกัน คือค่า retry_after ของ connection คิว มันคือเวลาที่คิวจะถือว่า "งานใบนี้คงตายไปแล้ว" แล้วเอากลับมาให้คนอื่นทำใหม่

// 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

ถ้าตั้ง retry_after=90 แล้วมีงานที่ใช้เวลาจริงสองนาที สิ่งที่เกิดคือพอถึงวินาทีที่ 90 คิวเด้งงานใบเดิมออกมาให้อีกตัวหนึ่งทำทั้งที่ตัวแรกยังทำอยู่ ผลคืองานเดียวถูกทำพร้อมกันสองที่ ซึ่งถ้าเป็นงาน outward ก็คือส่งซ้ำตรง ๆ และที่แย่กว่านั้นคือมันจะเกิดเฉพาะกับงานที่ช้าผิดปกติ แปลว่ามันจะโผล่มาตอนระบบกำลังมีปัญหาอยู่แล้วพอดี

ค่าพวกนี้เป็นค่าระดับ connection แยกรายงานไม่ได้ ถ้าในระบบมีงานที่ยาวมากอยู่ตัวเดียวแล้วที่เหลือสั้นหมด ทางที่ผมทำคือแยก connection ให้งานยาวไปอยู่ของมันเอง แล้วตั้ง retry_after ยาว ๆ เฉพาะเส้นนั้น ไม่ใช่ดันค่าของทั้งระบบขึ้นตามงานที่ช้าที่สุดใบเดียว เพราะทำแบบนั้นแล้วงานที่ตายจริงจะค้างนานเกินจำเป็นไปด้วยกันหมด

Cache::lock ต้องยาวกว่าเวลาที่งานใช้

ถ้าใช้ lock กันงานซ้อนอีกชั้น กติกาเดียวกันเป๊ะ — อายุของ lock ต้องมากกว่าเวลาที่งานใช้จริง ไม่ใช่มากกว่าเวลาที่เราคิดว่ามันควรใช้

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

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

จุดที่คนพลาดคือตั้ง lock ไว้ 60 วินาทีเพราะ "ปกติมันเสร็จใน 5 วิ" แล้ววันที่ปลายทางช้า งานใช้เวลา 90 วินาที lock หมดอายุตั้งแต่วินาทีที่ 60 ตัวที่สองเข้ามาได้ แล้วสองตัวยิงพร้อมกัน ส่วนตัวผมยึดว่าlock ต้องยาวกว่า timeout ของ process เสมอ ตั้งเผื่อไว้แล้วเสียแค่รอ ดีกว่าตั้งสั้นแล้วเปิดช่อง

failed() ห้ามทับสถานะที่สำเร็จไปแล้ว

ข้อสุดท้ายเป็นข้อที่ผมเองก็เคยเขียนผิดมาแล้ว ตัว failed() ของ job ดูเหมือนที่ที่เหมาะจะบันทึกว่างานล้มเหลว แต่มันถูกเรียกในจังหวะที่เราควบคุมไม่ได้

// ผิด — งานที่เสร็จไปแล้วรอบแรก อาจโดนเขียนทับเป็น 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)]);
}

สถานการณ์คืองานยาวที่โดนคิวเด้งซ้ำตาม retry_after — รอบแรกทำสำเร็จช้าไปนิด รอบสองล้มเหลว แล้ว failed() ของรอบสองมาเขียนทับสถานะที่รอบแรกเขียนว่าสำเร็จไปแล้ว ผลคือระบบรายงานว่าล้มเหลวทั้งที่ของถึงลูกค้าเรียบร้อยแล้ว แล้วก็จะมีคนไปกดส่งใหม่ตามรายงานนั้น

ตารางเช็กก่อนปล่อยงาน outward ขึ้นระบบ

ผมทำเป็นรายการสั้น ๆ ไว้ไล่ทุกครั้งที่มีงานประเภทนี้เพิ่มเข้ามา ใช้เวลาไม่ถึงสิบนาทีแต่กันเคสที่ตามเก็บไม่ได้ออกไปเกือบหมด

1. จองสถานะด้วย UPDATE เดียวก่อนยิง แล้วเช็กว่าได้ affected เท่ากับ 1 จริง ถ้าได้ 0 ต้องหยุด ไม่ใช่ทำต่อ
2. เขียนผลกลับต้องมีเงื่อนไขว่าแถวยังเป็นของเราอยู่ ถ้าไม่ใช่ แปลว่ามีคนแทรก ต้องจัดการไม่ใช่เงียบ
3. ยกเลิกต้องลบของจริงที่ปลายทางก่อน ลบไม่สำเร็จห้ามรายงานว่ายกเลิกแล้ว
4. เทียบ retry_after กับเวลาจริงของงานที่ยาวที่สุด ไม่ใช่เวลาเฉลี่ย
5. อายุ lock ต้องยาวกว่า timeout ของ process ที่ถือมันอยู่
6. failed() ต้องมีเงื่อนไขกันทับสถานะที่สำเร็จไปแล้ว
7. ทดสอบด้วยการฆ่า worker กลางคันจริง ๆ อย่างน้อยหนึ่งรอบ แล้วดูว่าแถวค้างอยู่ในสถานะอะไร

ข้อสุดท้ายเป็นข้อที่คนข้ามบ่อยที่สุดและมีค่าที่สุด ผมใช้วิธียิงงานทดสอบเข้าไปหนึ่งใบ รอให้มันเริ่มยิง แล้ว kill -9 ตัว worker ทิ้ง จากนั้นค่อยไปดูว่าตารางค้างอยู่ในสถานะไหน แล้วเปิด worker กลับมาดูว่ามันตัดสินใจถูกหรือเปล่า

# ทดสอบเส้นทางที่พังจริง — ทำกับปลายทางทดสอบเท่านั้น ห้ามยิงของจริง
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"

ถ้าผลที่ได้คือแถวค้างเป็น sending แล้วมีคนต้องไปเช็กเอง ผมถือว่าผ่านครับ นั่นคือพฤติกรรมที่ต้องการ ไม่ใช่ความล้มเหลว สิ่งที่ต้องไม่เกิดคือมันขึ้นสถานะสำเร็จสองรอบ หรือมี external_id สองตัวจากงานใบเดียว

ใครมีเงื่อนไขต่างจากนี้ก็ปรับตามสะดวกครับ แต่กติกาข้อเดียวที่ผมไม่ยอมยืดหยุ่นเลยคือห้ามยิงของออกไปก่อนที่ระบบจะจดว่ากำลังยิง ลำดับนี้ผิดเมื่อไหร่ที่เหลือจะเป็นการตามแก้ทั้งหมด ตรงไหนผมอธิบายตกหล่นหรือพิมพ์ผิด เดี๋ยวกลับมาแก้เพิ่มให้ภายหลังครับ