esm アジャイル事業部 開発者ブログ

永和システムマネジメント アジャイル事業部の開発者ブログです。

仕事で遭遇したRailsのバグにパッチを送るまでの話

こんにちは、@color_boxです。 仕事でRailsを使っている時にバグに遭遇したのでそれに対してパッチを送りました。

送ったパッチはこちらです。 github.com この記事ではバグ発見からパッチ送信までの過程について書こうと思います。

PRに至った経緯

PRに至るまでの流れを書いていきます。

Rack::Attackとそれによって発生するinodeの問題

私が仕事で開発しているアプリケーションではRack::AttackのThrottling機能を使用していました。 Rack::AttackKickstarterの出しているgemで、DDos攻撃に対する検知/アクセス遮断等を行えます。 github.com

Rack::AttackのThrottlingは1回のアクセスに対してアクセス元IPと時刻の記録を行います。 記録先はRedisやActiveSupport::Cache::Storeを選択可能です。

件のアプリケーションでは保存先としてActiveSupport::Cache::Storeのうちローカルファイルとして保存するActiveSupport::Cache::FileStoreが選択されていました。 この設定では、Rack::AttackActiveSupport::Cache::FileStoreを使用してRailsアプリのtmp/cacheディレクトリにデータを書き出していきます。

そして、この状態で長期間運用していると問題が発生します。 ファイル数が際限なく増えていき、inodeが枯渇してしまうのです。 inode枯渇すると最悪サービスが止まってしまうため対策が必要です。

inode枯渇対策とバグの発見

対策として、定期的にtmp/cacheディレクトリ内のキャッシュファイルを削除することにしました。 ActiveSupport::Cache::FileStoreのコードを読むと期限切れのキャッシュを削除するRails.cache.cleanupというメソッドがあり、これが使えそうです。

def cleanup(options = nil)
  options = merged_options(options)
  search_dir(cache_path) do |fname|
    entry = read_entry(fname, **options)
    delete_entry(fname, **options) if entry && entry.expired?
  end
end

https://github.com/rails/rails/blob/v6.0.3.2/activesupport/lib/active_support/cache/file_store.rb#L42-L49

ステージング環境でテスト的に試したところ、エラーが発生して利用できませんでした。 コードを読む限り正常に動作しそうですが、何故でしょうか? エラーを調べるとRails.cache.cleanupメソッドの下記コードのentry.expired?の箇所でNoSuchMethodErrorが起きていることがわかりました。

delete_entry(fname, **options) if entry && entry.expired?

このentryオブジェクトはread_entryメソッドによってtmp/cacheディレクトリから読み込まれたActiveSupport::Cacheインスタンスオブジェクトです。 cleanupメソッドは、tmp/cacheディレクトリにあるファイルから作成されたオブジェクトは全てActiveSupport::Cacheインスタンスオブジェクトであり、expired?メソッドを呼び出し可能であると捉えています。

なので、もしexpired?メソッドに対応できないオブジェクトがtmp/cacheディレクトリ内に保存されてしまうとcleanup呼び出し時にエラーとなるわけです。

では、このcleanupメソッドが想定していないキャッシュはどこから来るのでしょうか? それはSprocketsが作成しているものでした。

Sprocketsもまたtmp/cacheディレクトリ内にキャッシュファイルを出力しており、それはActiveSupport::Cache::FileStoreを使用せずに出力されます。 なので、もしこのファイルを読み込んでオブジェクトを作成しても、expired?メソッドは反応できず、NoSuchMethodErrorが発生するわけです。

PRの提出

エラーを見つけたのでテスト追加と挙動修正を行いPRを出します。 PRを出した後、マージされるまでのやり取りはPRにあるので記事上では割愛します。

github.com

PRを出す際のコメントなどは@koicさんに相談させていただきました。

余談ですが、個人的にはコードの修正よりもテスト追加の方が大変でした。 Railsの中でRails由来ではないキャッシュファイルを作成する必要があったからです。

まとめ

Railsのドキュメントやソースを読んでいると自分の目的にドンピシャな機能があったりします。 想定通りの動きをしない時は自分の環境かコードの少なくともどちらかがおかしいので徹底的に調査するとコントリビューションチャンスが生まれることがあります。 もし生まれなくてもコードへの理解が深まります。

なにかおかしいと感じたことがあれば、納得するまでコードを追ってみると、良い事が起こります。

この記事がどなたかの参考になれば幸いです。