Ruby / Rails関連

2023.01.13

Rails: 5年前のアドバイザリーロック実装が突然おかしくなった話（翻訳）

hachi8833

シェア
ツイート
ブックマーク
LINE

概要

元サイトの許諾を得て翻訳・公開いたします。

英語記事: How we got struck by 5–year–old implementation | Arkency Blog
原文公開日: 2022/11/15
原著者: Szymon Fiedler
サイト: Arkency Blog

日本語タイトルは内容に即したものにしました。

Rails: 5年前のアドバイザリーロック実装が突然おかしくなった話（翻訳）

最近私たちは、アドバイザリーロック（勧告的ロック）を取得するときのロックキーの算出部分に間違いを発見しました。これについては過去のreadモデルの構築記事を更新する形でカバー済みですが、この問題の全容をお伝えすることは皆さんにとっても興味深いものになるだろうと思いました。

アドバイザリーロックが必要になる理由

以下の非同期イベントハンドラをご覧ください。ここには以下の2つのコンテキストがバインドされています。

Banking: サードパーティサービスの銀行口座（bank account）の技術情報を管理する。
Accounting: 簿記業務（初期残高とその後の残高変化を特定の勘定科目（account）に入れるなど）のコンテキストにおいて、銀行口座に関心を持つ。

Bankingに銀行口座が作成されると、その銀行口座をAccountingに反映し、その銀行口座の簿記id（bookkeeping identifier）を提供する必要があります。これは、「すべての銀行口座は親コード512000を持ち、それに続く形で512101、512102、512103などのコードも持つ必要がある」といった特定のルールに基づいて行われます。

# frozen_string_literal: true

module Accounting
  class CreateBankAccount < Infra::EventHandler
    def call(event)
      tenant = ::Account.find(event.data.fetch(:tenant_id))
      bank_account = ::BankAccount.find(event.data.fetch(:id))
      bookkeeping_account = BookkeepingAccount.for(bank_account)

      BankAccount.create!(
        tenant: tenant,
        name: "#{bookkeeping_account.code} -- Bank",
        display_name: "Bank",
        code: bookkeeping_account.code, # 例: 512101
        parent_code: bookkeeping_account.parent_code, # 例: 512000
      )
    end
  end
end

上のコードはコンカレント処理で問題が発生しやすくなります。このビジネスロジックでは、指定のテナントに対して同じcodeで2つのAccounting::BankAccountを作成することは許されません。じきにActiveRecord::RecordNotUniqueエラーが発生するのは明らかです。

アドバイザリーロックを導入した

私たちが2017年に別のプロジェクトを手掛けていたときに、アドバイザリーロックを用いて悲観的ロック、具体的にはpg_advisory_xact_lock(key bigint)を実装してはどうだろうと思い付きました。

pg_advisory_xact_lockの動作はpg_advisory_lockと同じですが、現在のトランザクションの終了時に自動的にロックが解放される点が異なるので、明示的なロックの解放はできません。

pg_advisory_lockは、アプリケーションが定義したリソースをロックします。キーは単一の64ビットキー値、または、２つの32ビットキー（この2つのキー空間は重複しないことに注意）によって識別されます。

pg_advisory_xact_lockに渡されるbig integerを生成する方法が必要でした。RubyのObject#hashドキュメントに「このオブジェクトのIntegerハッシュ値を生成する」と書かれているので、これを用いるのが自然に思えました。

この仮説を手っ取り早く検証してみましょう。

uuid = "a2e920fd-c51a-44a8-924d-5dc6aaba9884"
lock_nr = uuid.hash
ActiveRecord::Base.transaction do
  puts "trying to obtain lock - #{Time.now}"
  ActiveRecord::Base.connection.execute "SELECT pg_advisory_xact_lock(#{lock_nr})"
  puts "lock granted, sleeping - #{Time.now}"
  sleep(50)
end
puts "lock released - #{Time.now}"

ロック1:

   (0.5ms)  BEGIN
trying to obtain lock - 2017-06-28 10:05:44 +0200
   (0.7ms)  SELECT pg_advisory_xact_lock(1924743033351481473)
lock granted, sleeping - 2017-06-28 10:05:44 +0200

ロック2:

trying to obtain lock - 2017-06-28 10:05:46 +0200
   (48570.8ms)  SELECT pg_advisory_xact_lock(1924743033351481473)
lock granted, sleeping - 2017-06-28 10:06:34 +0200

概念実証はうまくいきましたので、これを実装してみましょう。

# frozen_string_literal: true

class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true

  def self.with_advisory_lock(*args)
    transaction do
      bigint = args.join.hash
      ApplicationRecord.connection.execute("SELECT pg_advisory_xact_lock(#{bigint})")
      yield
    end
  end
end

module Accounting
  class CreateBankAccount < Infra::EventHandler
    def call(event)
      tenant = ::Account.find(event.data.fetch(:tenant_id))
      bank_account = ::BankAccount.find(event.data.fetch(:id))

      ApplicationRecord.with_advisory_lock(tenant.id) do
        bookkeeping_account = BookkeepingAccount.for(bank_account)

        BankAccount.create!(
          tenant: tenant,
          name: "#{bookkeeping_account.code} -- Bank",
          display_name: "Bank",
          code: bookkeeping_account.code,
          parent_code: bookkeeping_account.parent_code,
        )
      end
    end
  end
end

テナントのスコープ内で一意性を保証する必要があったので、big integerの算出ではtenant_idを入力値として受け取りました。

コンカレンシーの問題をテストする

2015年にRobertがRailsアプリの競合状態をテストする記事を書いていたので、私たちは既にコンカレントなコードのテスト方法について十分理解しているはずですよね？

# frozen_string_literal: true

require_relative "test_helper"
require "database_cleaner/active_record"

module Accounting
  class OnBankAccountCreatedConcurrencyTest < TestCase
    self.use_transactional_tests = false

    setup { DatabaseCleaner.strategy = [:truncation] }

    def test_concurrency
      begin
        concurrency_level = ActiveRecord::Base.connection.pool.size - 1
        assert concurrency_level >= 4

        bank_accounts = concurrency_level.times.map { create_bank_account }

        fail_occurred = false
        wait_for_it = true

        Thread.abort_on_exception = true
        threads =
          concurrency_level.times.map do |i|
            Thread.new do
              true while wait_for_it
              begin
                Accounting::CreateBankAccount.new.call(
                  Banking::BankAccountCreated.new(data: { tenant_id: 2137, id: bank_accounts.fetch(i).id }),
                )
              rescue ActiveRecord::RecordNotUnique
                fail_occurred = true
              end
            end
          end
        wait_for_it = false
        threads.each(&:join)

        refute fail_occurred
        assert_equal 4, Accounting::BankAccount.of_tenant(2137).where(parent_code: 512_000).size
      ensure
        ActiveRecord::Base.connection_pool.disconnect!
      end
    end

    teardown { DatabaseCleaner.clean }

    private

    def create_bank_account
      BankAccount.create!(
        connector_id: 12_345,
        balance_currency: Money::Currency.new("EUR").iso_code,
        balance_value: 1_000_000.00,
        external_id: SecureRandom.uuid,
      )
    end
  end
end

このテストはパスし、コードが正しく動いていることがわかったので、コードをproduction環境でリリースしてぐっすり眠りにつきました。

忘れた頃に突然おかしくなった

数年後、ActiveRecord::RecordNotUniqueエラーという手痛いしっぺ返しを食らいました。私たちはこの原因について俄然興味を惹かれましたが、手がかりはまったくつかめませんでした。このコードはSidekiqで非同期に実行され、失敗するとリトライしたので、問題は自己修復されていたのです。簡易調査では答えが見つかりませんでした。これはアプリでは問題にはならなかったものの、Honeybadgerで頻発しており、そのたびに開発者たちは頭を抱えました。

そのとき、ふとチームメンバーの1人の頭に過去のプロジェクトの記憶がよぎりました。当時そのプロジェクトでは、同じような方法でアドバイザリーロックを取得すると、時たま同じような問題が発生していたというのです。ここ数か月の間に両プロジェクトの技術スタックで何か相違点がなかったかどうかについて議論を始めました。

「あ、そういえばSidekiqのプロセスを1個追加してたわ」

ただちにirbプロセスを2つ個別に立ち上げて、それが原因かどうかを調べてみました。

プロセス1:

irb(main):001:0> 123456.hash
=> -169614201293062129

プロセス2:

irb(main):001:0> 123456.hash
=> -4474522856021669622

「これだ！ドッカン爆発真っ黒け...」

ロックキーを「正しく」算出する

advisory_lockメソッドの当初の実装は、異なるMRIプロセスに一意のハッシュ値を提供しておらず、コードがActiveRecord::RecordNotUniqueエラーを連発していました。

# frozen_string_literal: true

class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true

  def self.with_advisory_lock(*args)
    transaction do
      bigint = args.join.hash
      ApplicationRecord.connection.execute("SELECT pg_advisory_xact_lock(#{bigint})")
      yield
    end
  end
end

以前のadvisory_lockが期待通りに動作しておらず、同じリソースを共有してしまったことを証明するには、2つの異なるデータベース接続を用いる2つの異なるプロセスが必要でした。そして開発環境もCIも、テストのセットアップがこの基準を満たしていませんでした。

RubyのObject#hashドキュメントに、この問題の核心となる重要な記述があります。

あるオブジェクトのハッシュ値は、Rubyの起動や実装ごとに同一であるとは限りません。Rubyの起動や実装の違いに影響されない安定したidが必要な場合は、ハッシュ値をカスタムメソッドで生成する必要があります。

これを修正するために、PostgreSQLデータベースにカスタムのhash_64()関数を作成しました。

create function hash_64(_identifier character varying) returns bigint
    language plpgsql
as
$$
DECLARE
hash bigint;
BEGIN
  select left('x' || md5(_identifier), 16)::bit(64)::bigint into hash;
  return hash;
  END;
  $$;

alter function hash_64(varchar) owner to dbuser;

続いて、このhash_64()関数を用いてadvisory_lockの実装を修正しました。

# frozen_string_literal: true

class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true

  def self.with_advisory_lock(*args)
    transaction do
      ApplicationRecord.connection.execute("SELECT pg_advisory_xact_lock(hash_64('#{args.join}'))")
      yield
    end
  end

hash_64()関数の実装は、Eventideのコードベースにあるものを利用しました。

ロックキーの算出をあくまでRubyでやりたいのであれば、他にもZlib#crc32を使う方法などがあります。

# frozen_string_literal: true
class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true

  def self.with_advisory_lock(*args)
    transaction do
      ApplicationRecord.connection.execute("SELECT pg_advisory_xact_lock(#{Zlib.crc32(args.join)})")
      yield
    end
  end

Rails: Webpackをesbuildに移行してJSのビルドを縮小・高速化（翻訳）

Rails: beforeバリデーションをやめてセッターメソッドにしよう（翻訳）

シェア
ツイート
ブックマーク
LINE

Twitter: @hachi8833、GitHub: @hachi8833 コボラー、ITコンサル、ローカライズ業界、Rails開発を経てTechRachoの編集・記事作成を担当。これまでにRuby on Rails チュートリアル第2版のコンテンツ監修、Railsガイドのコンテンツ作成を担当。かと思うと、正規表現の粋を尽くした日本語エラーチェックサービス enno.jpを運営。実は最近Go言語が好きで、Goで書かれたRubyライクなGoby言語のメンテナーでもある。仕事に関係ないすっとこブログ「あけてくれ」は2000年頃から多少の中断をはさんで継続、現在はnote.comに移転。Amazonウィッシュリスト: https://bit.ly/32aAmiI

Rails: 5年前のアドバイザリーロック実装が突然おかしくなった話（翻訳）

Rails: 5年前のアドバイザリーロック実装が突然おかしくなった話（翻訳）

アドバイザリーロックが必要になる理由

アドバイザリーロックを導入した

コンカレンシーの問題をテストする

忘れた頃に突然おかしくなった

ロックキーを「正しく」算出する

関連記事

【速報】Rails 8.0.0がリリースされました

Rails 8: strong parametersの新しいparams.expectの使い方（翻訳）

Rails: DBメンテナンス支援ツール "maintenance_tasks" README（翻訳）

週刊Railsウォッチ: Rails 8.0 Beta 1リリース、Railsのメンテナンスポリシー更新ほか（20241024）

Rails: システムテストのドライバをSeleniumからPlaywrightに差し替えたら安定した（翻訳）

Rails: Active Storageで知っておくべきアドバイス集（翻訳）

Ruby 3.4に導入される次世代の帯域外ガベージコレクション（翻訳）

Safari 17.5で多数のリダイレクト画像にloading="lazy"を指定すると読み込まれない問題（翻訳）

Tailwind: メディアクエリに垂直方向のスクリーンサイズを追加する（翻訳）

複雑なファイル処理をサーバーレスでシンプルにする方法（翻訳）

関連記事

CONTACT

Rails: 5年前のアドバイザリーロック実装が突然おかしくなった話（翻訳）

Rails: 5年前のアドバイザリーロック実装が突然おかしくなった話（翻訳）

アドバイザリーロックが必要になる理由

アドバイザリーロックを導入した

コンカレンシーの問題をテストする

忘れた頃に突然おかしくなった

ロックキーを「正しく」算出する

関連記事

【速報】Rails 8.0.0がリリースされました

Rails 8: strong parametersの新しいparams.expectの使い方（翻訳）

Rails: DBメンテナンス支援ツール "maintenance_tasks" README（翻訳）

週刊Railsウォッチ: Rails 8.0 Beta 1リリース、Railsのメンテナンスポリシー更新ほか（20241024）

Rails: システムテストのドライバをSeleniumからPlaywrightに差し替えたら安定した（翻訳）

Rails: Active Storageで知っておくべきアドバイス集（翻訳）

Ruby 3.4に導入される次世代の帯域外ガベージコレクション（翻訳）

Safari 17.5で多数のリダイレクト画像にloading="lazy"を指定すると読み込まれない問題（翻訳）

Tailwind: メディアクエリに垂直方向のスクリーンサイズを追加する（翻訳）

複雑なファイル処理をサーバーレスでシンプルにする方法（翻訳）

関連記事

Ruby 3.4に導入される次世代の帯域外ガベージコレクション（翻訳）

週刊Railsウォッチ: Rails公式のdevcontainerでKamalをサポート、RailsBump.orgほか（20241108）

Ruby 3.3.6がリリースされました

CONTACT