べき等なジョブを書きたい

Make your job idempotent and transactional

Sidekiqのベストプラクティスを参照すると、「ジョブはべき等でトランザクショナルなものにする」と定義されている。 できることならそうしたいものなのではあるが、例示されているコードは以下だ。

def perform(card_charge_id)
  charge = CardCharge.find(card_charge_id)
  charge.void_transaction
  Emailer.charge_refunded(charge).deliver
end

あくまでこれは想像に過ぎないのだけれども、あるクレジットカードに対して返金を行ってから返金した文言を記載したメールを飛ばしているのだと思う。 ここで問題になるのはAPI経由などで返金処理が行われたあとにEmailerクラスがなんらかのエラーを発生してしまった場合にSidekiqはリトライを行うはずだ。 もしその時void_transactionの中身が再び返金処理を行ってしまったらと考えると恐ろしい。

金銭を扱うようなWebアプリケーションではなくとも、正しくないジョブのデザインはリソースを無駄に消費したり後々の整合性に影響が出るかもしれない。 私もほぼすべてのアプリケーションでSidekiqを使っているけれども、今日は特にそのことを意識したと思う。 普段はリトライをしないようにオプションに設定したり、SQLiteのコネクションエラーを回避するのにリトライを意図的に発生させるような使い方ばかりだった。

まず先程の例示したコードを修正するのであればまずはEmailerクラスのメール送信をdeliver_laterに書き換えるかもしれない。 こうすれば仮にEmailerがエラーを発生させたとしてもリトライはEmailerクラスで行うのだからvoid_transactionには影響がでないと思う。

あるいはCardChargeクラスにtransaction_statusなどのカラムを用意してvoid_transactionの内部でステータスを変更する方法が思いつく。 私はこの方法を採用した。

module Downloadable
  extend ActiveSupport::Concern

  included do
    enum download_status: { 
      idle: 0,
      downloading: 1,
      errored: 2,
      downloaded: 3
    }
  end

  # 他のクラスからはこのメソッドを使う
  def download_later
    DownloadJob.perform_async(id)
  end

  def downloadable?
    idle? || errored?
  end

  def download!
    downloading!
    download
    downloaded!
  rescue StandardError => e
    Rails.logger.error e.message
    errored!
  end

  private

  def download
    # 実際のダウンロード処理
  end
end

概ねこのような感じだ。 続いてDownloadJobはこうなっている:

class DownloadJob
  include Sidekiq::Job

  def perform(id)
    download_file = DownloadFile.find(id)
    download_file.download! if download_file.downloadable?
  end
end

DownloadFileは先程のDownloadableをインクルードしているクラスで任意のファイルをダウンロードできる。 以前別のブログで極力SidekiqのJobに処理を書くのはやめてクラスで書くように心がけていることを書いたのだけれども、ステータスがdownloadable?のときだけdownload!できるように変数に書き換えている。 本当は1行で書きたかった。 download!ではすでに6行書いてしまっているのでこうせざるを得なかった。 downloadがプライベートなのも正直使い勝手はあまりよくないと思う。 本当ならdownload!は強制的にダウンロードさせるという意味合いにして、downloadをパブリックにしてdownload!をラップしてもよかったかもしれない。

まだ手を加える余地が残っているものの、このコードはなかなか気に入っている。 ランダムにテストをエラー発生させたり、ダウンロード中の状態やボタン連打などを繰り返しても最終的にはerroredかdownloadedに落ち着いた。

現在はGraphQLを重点的に書いているが、従来のRESTのフォームで仮に送信ボタンを連打されても安全だと思う。