Ruby 2.7.0でキーワード引数として渡された引数なのかどうかフラグを確かめる方法

class Hash
  class << self
    def ruby2_keywords_hash?(hash)
      !new(*[hash]).default.equal?(hash)
    end

    def ruby2_keywords_hash(hash)
      _ruby2_keywords_hash(**hash)
    end

    private def _ruby2_keywords_hash(*args)
      args.last
    end
    ruby2_keywords(:_ruby2_keywords_hash) if respond_to?(:ruby2_keywords, true)
  end
end


RUBY_VERSION # => "2.7.0"

def passed_kw?(*args)
  Hash.ruby2_keywords_hash?(args.last)
end
ruby2_keywords(:passed_kw?) if respond_to?(:ruby2_keywords, true)

passed_kw?({ a: 1 }) # => false
passed_kw?(a: 1) # => true

hash = { a: 1 }
passed_kw?(hash) # => false
passed_kw?(**hash) # => true

kw = Hash.ruby2_keywords_hash(hash)
passed_kw?(kw) # => true
passed_kw?(**kw) # => true

説明書こうとしたけどだいぶめんどくさかったのでコードで感じ取ってほしいんですが、Ruby 2.7.0以降Ruby 3に向けてキーワード引数渡しの非互換をハンドリングするためにruby2_keywordsを使ってキーワード引数渡しされたhashオブジェクトにフラグを付けることができる、というかRuby 2.6以前とRuby 2.7以降の両方サポートしたいライブラリメンテナはこのフラグを付けて回らなければRuby 3ではArgumentErrorでお前はもう死んでいる状態になっています。

フラグが付けれるのはいいとしてRuby 2.7.0ではこのフラグが付いてるhashオブジェクトなのかそうじゃないhashオブジェクトなのか確かめる方法が公式には提供されてなくて、どこで呼び出されたときにフラグを付ける必要があって、ちゃんとフラグが付いたままお届け先のメソッドまでたどり着いてるのかのデバッグが死ぬほどめんどくさかった。

これはmameさんのハックを見て知った方法で、ようはフラグが付いてるhashオブジェクトかそうじゃないオブジェクトかで違う振る舞いをする処理を通らせてその結果を観測することでどっちだったかを確かめるという方法です。

このフラグが付いてるhashオブジェクトかそうじゃないオブジェクトかで違う振る舞いをする処理がRubyの世界からはほとんど存在しないので、普段Rubyでコードを書いてる常人が自力では気づけんやろって方法で、一部のメソッド(initializeとかmethod_missingとか)をRubyのコードから直接呼び出すんじゃなくてCのコードから間接的に呼び出されるときにフラグが付いてるhashオブジェクトをdupするので、dupされずにそのままのオブジェクト(object_id)だったらフラグが付いてなかったってことでdupされて別のオブジェクトが観測されたらフラグが付いてたオブジェクトだったという技を使っています。

いろいろ試行錯誤してみてRuby 2.7.0の時点ではnewを通さないとargs lastにkwargs flagが付いてるかどうかRubyの世界からは確かめようがないということが分かったけどこれはマジで答えを先に知ってないと気づきようがない無理ゲーすぎた

— Ryuta Kamizono (@kamipo) January 19, 2020

原理が分かったので既存のメソッドでこの用途に丁度いいメソッドを探した結果、Hash.newにhashのデフォルト値としてフラグ付きかもしれないhashオブジェクトをsplat渡ししてHash#defaultで取り出して観測するという方法が一番シンプルであろうというところに至り、この技を使ってRailsでもキーワード引数完全分離への対応を進めています。

github.com

このフラグ付きかどうか確かめる方法があるのかないのか常人では思い至らん問題にライブラリメンテナは直面すると思いますが、Ruby 2.7.1にはフラグ付きかどうか確かめる方法が公式にバックポートされる予定なので、これで警告が多い日も安心ですね。

github.com

ISUCON9予選ふりかえり

なにもできなかったし書くことないわーって思ってたけど、チームのふりかえりをGitHubのissueに書いてたらふつうにこれ外に書けばいいなってなったのでここに記す。

• アクセスログから得られる情報の解像度が相対的に低くなってきたと感じた。ざっくりどのエンドポイントが遅いとか何回叩かれてるとかの概観を知るのは依然として重要だけど、近年は遅いエンドポイントの処理はめちゃくちゃ複雑になってきていて、たとえば/buyが遅いぐらいの情報の解像度では情報量が足りてなかった。
• 外部APIへのリクエストが絡むような問題への心の準備とかできてなかった。なんなら準備してきてないし予選では出ないでほしいなとか思っていた。
• プログラマとしての筋力が足りてなかった。たとえばアクセスログからの情報では足りないってなったときにときにじゃあやばそうなところ全部にprint文書いて測るわ！ぐらいの気概が必要だった。外部リクエストが遅いとかロック競合で時間がかかっていると遅いけどあまりリソースは食ってないように見えるしそういう面でもprint文書いてでも知りたい情報ログに出したるわぐらいの気概が必要だった。
• 外部APIへのリクエストが遅いかもってなったときに遅いリソースへのリクエストを多重化するようなプログラミングを普段してなかったのでひよった。きれいに言語のAPIやライブラリが使えなくてもたとえばThread.newしまくってでもやったるわぐらいの気概が必要だった。
• もはやデータベースに入ってるデータだけじゃなくて外部APIのレスポンスも含めてアプリケーションを構成するデータソースだという考えで挑まなければならないと思った。実際外部APIのレスポンスの内容がリクエストに対して不変な内容かそうでないか、不変ならキャッシュ可能かという考えに至れていなかった(もしキャッシュ可能だと分かれば不慣れなリクエスト多重化プログラミングを避けることができたかもしれない)。
• 自分たちに相性がいい過去問で練習してできることをやればいけそうって過信してしまった。苦手だった問題も過去にはあるのにそういう苦手分野に向き合うことをおざなりにしてしまったところはいなめなかった。

ISUCON9予選、手も足もでなかったけど、いい問題だと思ったし楽しめた。近年データベースがボトルネックじゃない問題で僕の楽しみがあまりないからISUCONから遠ざかってたところあったけどISUCON8予選がデータベースのロック競合を出題に盛り込んできて、そういう問題ならやってみたかったって思ったし、今回の問題は去年の予選問題のようでもありそこからさらに去年の本選問題を足してひねりが加えられたような一筋縄ではいかない感じであった。今回誘ってくれたid:Soudaiさんと一緒に戦ってくれたid:sugyanにまじ感謝、楽しかったです。

Rails 6.0の複数DBでリードレプリカのテストするのたぶん大変

Rails 6.0の複数DBのレビューしてるときに気づいたことなんですけど、たぶんリードレプリカからデータを読むテストをするのたぶん大変だと思われます。

うちの業務のアプリでActive Recordが更新を検知できない方法でデータが更新されるとテストがコケるという問題が以前にあり、これと同じ構造の問題がマスターのコネクションで更新したときマスターのコネクションのクエリキャッシュはクリアされるけどリードレプリカのコネクションのクエリキャッシュは残ったままというのがあるよね、というのをテストコードで示そうと思ったときのことである。

github.com

通常RailsアプリでDBつかったテストをするとき、テストの中で変更されたデータを毎回初期状態に戻すのにフィクスチャーをロードし直すのは時間がかかって効率がわるいので、テストケースに入る前にトランザクションを開始しといてテストケース終わったらロールバックして変更をなかったことにすることで初期状態に戻し、時間のかかるフィクスチャーロードし直しを避けるということが効率のために行われています。

この、効率のためにテストケース終わったらロールバックして変更をなかったことにするためにテスト内のトランザクションはネストしたトランザクションになっていて実際にコミットは起きないということが複数DBのリードレプリカと相性がとてもわるい。実際にコミットが起きないことにはリードレプリカに更新がレプリケートされてこないのでずっと時が止まった状態なのである。

あたかもマスターで更新が起きたっぽいときにリードレプリカにも更新が伝搬しているかのように見せかけるため、Active Recordはテストのときデフォルトですべてのコネクションプールをマスターのコネクションプールにすり替えるということで対処している。コミットが起きないんだったら全部マスターに接続してテストすればいいじゃないってやつです。しかしこれをされるとマスターではなくリードレプリカからデータを読んでることをテストしたいときにめっちゃこまるという話である。

github.com

コード読んだ限りではフィクスチャー毎回読み直しモードのときは setup_shared_connection_pool は呼ばれないから大丈夫そうに見えるけど、自分が試したときにはフィクスチャー毎回読み直しモードのときでも setup_shared_connection_pool の効果を自力で打ち消さないとまったくリードレプリカに接続できなくてめっちゃ苦労したので、先人の記憶としてここに書き記しておきます。同じ問題にぶち当たったりぶち当たらなかったりなんか分かった人いて教えてくれたらここに追記します。

あと、リードレプリカでクエリキャッシュ残る問題はがんばってテスト書いて修正される運びとなったのでこれで更新の多い日も安心ですね。

github.com

Rails 6.0でDeprecatedになるActive Recordの振る舞い3つ

Deprecatedにした経緯というか背景が伝わってるのかどうかアレだと思ったので、ここに日本語にて書き記しておく。

• Deprecate mismatched collation comparison for uniquness validator by kamipo · Pull Request #35350 · rails/rails · GitHub

Active Recordのuniqueness validatorはデフォルトでcase sensitiveな比較をするんですが、これが、文字列のデフォルトのcollationがcase insensitiveなMySQLと相性が悪く、DB上のUNIQUE制約と一致しない振る舞いだったりINDEXが効率よく使えずDBが死ぬみたいな問題を引き起こしていました。

例: 本当にあったRailsの怖い話

僕も主に仕事のコードレビューで過去に何度もこの問題を指摘してきたわけですが、去年ぐらいにmakimotoさんにこれRails側でなんとかならないんですか的なこと言われてそりゃ同僚にRailsコミッターおったら仕事で遭遇したRails側のあらゆる問題はRails側でなんとかなってほしいわなって思ったので気合いでなんとかすることにしたという次第です。

いないとは思いますが、MySQLなどでデフォルトのcollationがcase insensitiveな文字列をあえてわざとcase sensitiveな比較をしていた奇特なユーザーの皆様は今後明示的に case_sensitive: true オプションを指定していただくことになります。

• Deprecate `where.not` working as NOR and will be changed to NAND in Rails 6.1 by kamipo · Pull Request #36029 · rails/rails · GitHub

where.not(a: ..., b: ...) の結果が where(a: ..., b: ...) の補集合になるようになります。逆にいうと6.1になるまでは where.not(a: ..., b: ...) の結果は where(a: ..., b: ...) の補集合になるとは限りません。

想像してほしいんですが、血液型B型の男性(私です)の集合があったとして、それのNOTを取った集合は血液型B型の男性以外になってほしいと思いませんか？僕は思います。現状、Active Recordの where.not は血液型B型の男性という集合を反転した結果として、男性でもなく血液型B型でもない集合を返します。もしこれが想像していた振る舞いと違う場合、アプリケーションは暗黙のうちにバグってる可能性があるので where.not を使っている皆さんは手元のコードをよく確認してみるのがよいのではないかと思います。

• Deprecate using class level querying methods if the receiver scope regarded as leaked by kamipo · Pull Request #35280 · rails/rails · GitHub

これは説明がむずかしいというか面倒なんですけど、たとえばTopicのツリー型の掲示板みたいなものがあったとして、トップレベルのTopic、かつその中でも子スレッドを持つものをscopeとして定義したいとします。

class Topic < ActiveRecord::Base
  scope :toplevel, -> { where(parent_id: nil) }
  scope :children, -> { where.not(parent_id: nil) }
  scope :has_children, -> { where(id: Topic.children.select(:parent_id)) }
end

# Works as expected.
Topic.toplevel.where(id: Topic.children.select(:parent_id))

# Doesn't work due to leaking `toplevel` to `Topic.children`.
Topic.toplevel.has_children

子スレッドを持つという条件を has_children として抽出して利用したいわけですが、これは現状ほぼ意図した通りには動きません。というのもscopeのメソッドチェインは条件が累積したrelationを伝搬させるのにクラスグローバルな状態を汚染する実装方法を現状とっており、scope定義内はその汚染されたクラスグローバルな状態の影響を受けるためです。

現在、我々がどのようにしてこの問題を回避しているかというと Topic.unscoped.children.select(:parent_id) のように unscoped をつけるのを忘れないように頑張る、というものですが、6.1からは unscoped をつけるのを忘れないように頑張らなくてもよくなるということになります。

逆にいうと、現状scope定義内でモデルクラスから直接scopeやquery methodsを呼び出してるケースではユーザーの期待に反して意図せず別のscopeの状態が注入されている可能性があるので、今一度手元のコードを確認されるのがよいかと思います。

以上、よろしくお願いいたします。

Rubyで安全な文字列リテラルかどうかを判別したい

Rails 5.2からRails SQL Injection ExamplesにあるようなSQLインジェクションを防ぐ仕組みが導入されて、Post.order(params[:order])みたいなコードは心温まる正規表現によるチェックをパスしないと危険とみなされるようになって、お前が安全やと思うんやったらPost.order(Arel.sql(params[:order]))しろってことになった(rails/rails#27947)。

これはRails 5.0のときにparamsがHashのサブクラスじゃなくなったときに比べればマシだけど、明らか安全やと思ってるリテラルもRailsに危険とみなされて既存のアプリケーションによったら非常にわずらわしい。たとえばDiscourseというRailsアプリは5.2に上げるときにこれの影響をモロに受けるんやけどっていうお気持ちを表明しています(rails/rails#32995 (comment))。

そこでこのわずらわしさを軽減する方法を考えたんですけど、もし#orderに渡される値がユーザーのリクエスト由来の未知の値でなくソースコード上に直に書かれた文字列リテラルだったとしたら、これは安全ってことにしていいと思いませんか？僕はそう思いました。

ソースコード上に直に書かれた文字列リテラルかどうかを完全に判別できる方法は今のところおそらく存在しないと思うけど、ユーザーのリクエスト由来じゃないかどうかをできるときだけできる範囲で区別するというのだと、# frozen_string_literal: trueのときにユーザーのリクエスト由来の値とちがって文字列リテラルはfrozenになるという性質が利用できそうです。という方法を実装したのが以下。

github.com

これにはひとつ物言いがついて、"#{user_input}"がfrozenにならんのやったらええけどそこがなー的なこと言われてマジカーってなってたら、interpolated stringを判別する方法を教えてくれる神が降臨した(rails/rails#33330 (comment))。

# frozen_string_literal: true

dynamic = DATA.read
constant = "bar"
interpolated = "#{dynamic} DESC"

puts "[dynamic] frozen: #{dynamic.frozen?}, interned: #{dynamic.equal?(-dynamic.dup)}"
puts "[constant] frozen: #{constant.frozen?}, interned: #{constant.equal?(-constant.dup)}"
puts "[interpolated] frozen: #{interpolated.frozen?}, interned: #{interpolated.equal?(-interpolated.dup)}"

__END__
foo

[dynamic] frozen: false, interned: false
[constant] frozen: true, interned: true
[interpolated] frozen: true, interned: false

パッと見なにを判別してるんかよくわからんかったので同僚のRubyコミッターに聞いたらプリントデバッグしながらいろいろ調べてくれた結果すべてを理解することに成功したのでその成果をここでシェアしたいと思います。

これはfrozen literalがRuby 2.5で導入されたfstring tableに登録されているものと同一かどうかを判別していて、constant literalだとfstring tableに登録されたインスタンスを返すけどinterpolated literalだとただのfrozenなだけのインスタンスになるという性質の違いを利用しています。String#-@がfstringを返すのでconstant.equal?(-constant.dup)はfstring tableからlookupしなおしたインスタンスが自分自身と同一だったら自分はfstringだったということでつまりconstant literalだったということがわかります。ちなみに.dupが必要なのはRuby 2.5では自分がfrozenだったらString#-@はfstringではなく自分をそのまま返してしまう問題があるからで、この問題はruby-headでは解決済みなので.dupは要らなくなります(ruby/ruby@256411b)。

この方法は判別しようとするinterpolated literalのバリエーションだけfstring tableに登録してしまうので、なんか他にリーズナブルな方法ないかなって調べてくれたんですけど、今のところなさそう、ユースケース次第ですかねハハハーって感じでお開きとなりました。

以上、Ruby 2.5以上で文字列リテラルを判別する方法でした。

MySQLのクエリの良し悪しはrows_examinedで判断する

仕事やらなんやらでMySQLのクエリの良し悪しを判断する必要があるとき、EXPLAINの内容だけだとどのぐらい良くなったり悪くなったのか分からないので SET long_query_time = 0; してrows_examined (そのクエリでrows_sent行の結果を返すために何行に触ったのか)も一緒に提示するようにしている(少なくともMySQL 5.7時点ではrows_examinedはslow_query_logでしか確認できないはずperformance_schemaが有効ならevents_statements_historyやその仲間たちで確認できるとのこと*1 MySQL :: MySQL 5.6 リファレンスマニュアル :: 22.9.6 パフォーマンススキーマステートメントイベントテーブル)。

例:

上の例のBeforeは、もともとDBAが書いた温かみのあるSQLでORDER BY LIMIT最適化を効かせてLIMIT 20されたクエリをORDER BY狙いのインデックスを狙って20行の結果を返すために20行だけに触れている、つまり最も良い実行計画のクエリが、Afterでは深遠な理由により別のテーブルの条件で絞り込んだりしたりしなかったりするためのJOINで最適化の狙いが効かなくなって同じ20行の結果を返すために259454行に触れてしまうようになってしまったの図です。

仕様を保ったまま最適化の狙いが効くようにクエリの書き変えを行うと、20行の結果を返すために駆動表と内部表それぞれ20行ずつだけ触るのが最も良い実行計画になるので、その修正案をrows_examinedと共に提示しているのが上の図です。

ゴルフで例えると、EXPLAINは大まかな方向性や飛距離を合わせるドライバーのような感じで、グリーンによせたあとカップを狙うパター的なのがrows_examinedという感じ。ドライバーでもパッティングできるけどパターも使いこなせるとスコアアップ間違い無し💪

*1:https://twitter.com/RDBMS/status/976955535076466688

ActiveRecordでINの中が一万個とかにならないようにする

この記事は MySQL Casual Advent Calendar 2017 の23日目の記事です。

みなさんORマッパーは使っていますか？

僕は仕事とか趣味でActiveRecordというORマッパーを使っているんですけど、こいつ例えば

Team.preload(players: :high_score).to_a

みたいなことをするとすぐ

SELECT `scores`.* FROM `scores` FROM `scores`.`id` IN (a, b, c, ...数千個続く...)

みたいなクエリを生成しよるんですけど、MySQL 5.7に上げたときに range_optimizer_max_mem_size の制限で実行計画がテーブルスキャンに落ちてえらい目にあったことがありました。MySQL側で range_optimizer_max_mem_size = 0 することでこの制限を無くすことができますが、MySQL側の設定によらずアプリケーションが動作できるようINの中の個数を制限する方法をここでは考えます。

MySQL以外のことはよくわからないんですが、どうやらOracleにはINの中の個数が1000個までしか入れられないという制限があるようで、これに対応するためにActiveRecordには in_clause_length の個数ずつにsliceしてクエリを投げる仕組みが元々存在します。Oracle以外には個数の制限はないんですけど、この値を上書きすると他のバックエンドでもINの中の個数を制限することができるわけです。例えばOracle同様1000個までに制限したい場合は以下のようにすればよいです。

ActiveSupport.on_load(:active_record) do
  module ActiveRecord
    module ConnectionAdapters
      module DatabaseLimitsExt
        def in_clause_length
          1000
        end
      end

      class AbstractAdapter
        prepend DatabaseLimitsExt
      end
    end
  end
end

もしみなさんのORマッパーがINの中が一万個とかになるクエリを生成しているなら range_optimizer_max_mem_size の制限に引っかかってないか確認してみるといいかもしれません。

【追記】

Rails 6.0.0以降sliceしたINをひとつのクエリで投げるのでここで期待した効果は得られなくなりました。

github.com