この記事はモチベーションクラウドアドベントカレンダー5日目の記事です。
こんにちは。リンクアンドモチベーション、モチベーションクラウド開発チームの江上です。
つい先日、約3ヶ月のエンジニア研修を卒業した19年新卒のメンバーを開発チームに迎え入れました。弊社では全社研修後、数ヶ月のエンジニア研修の後エンジニアやプロダクトオーナー、カスタマーサポートなどのプロダクトチームに配属されるフローになっていますので、例年この時期に配属になります。
ありがたいことに、現在モチベーションクラウドは大手のお客様にも使っていただけるプロダクトに成長しました。それに伴い、性能やセキュリティにおいて求められる基準も高くなってきています。そこで、既存メンバーの復習も兼ねて、新人であったとしても最低限押さえておくべきことを、具体例を交えて開発開始前にインプットすることにしました。
同じく本番コードを初めて書く新人を迎え入れる方、今から本番コードを書き始めようという方、クリスマス前に初心にかえって、清らかな心を取り戻したい方の参考になれば幸いです。
0. 前提条件
弊社では、以下の事項を習得していることを研修卒業の条件とするプログラムを実施しています。
- SQLの基本習得
- DockerかVMをつかった開発環境の構築習得
- Rails Tutorial最低1周
- rspecの書き方習得
- Gitの使い方習得
- エディタの使い方習得
1. 事前知識
1-1. 読本
あくまで本番コードを書く上で、以下の資料を事前に読むことを決めています。
読むだけではなく、自分なりの感想文（まとめ）を提出してもらっています。
たまに、先輩からのいくつかクイズを出されます。
- リーダブルコード
- 安全なwebアプリケーションの作り方
- モチベーションクラウドのマニュアル
1-2. 開発中の注意事項
1-2-1. 開発に必要な画面を常に開いておくこと
なにか問題があっても、logを見ていなかったり、consoleで試していないと勘違いで進んでしまう可能性があります。
- ググるためのchrome
- 試すためのrails console
- 確認するためのlog
は常に用意しておきましょう。
1-2-2. 作業ブランチは最新にすること
朝来たら、 git pull や git merge 親ブランチ をしましょう。
特に、pushするときになってmergeすると、コンフリクト地獄で死にたくなります。
2. パフォーマンスについて
全てを網羅できているわけではないですが、抑えておいてほしいところを列挙しました。
2-1. N+1を起こさない
N+1問題とは
ループ処理の中で都度SQLを発行してしまい、大量のSQLが発行されてパフォーマンスが低下してしまう問題のことです。
例えば、1回1000件取るのに300msで終わるのに、1000回1件ずつ取ると、30ms * 1000 = 30000msかかってしまったりします。
前提
class ModelA < ActiveRecord::Base
has_many model_bs
end
class ModelB < ActiveRecord::Base
belongs_to model_a
end
解消方法
2-1-1. includesする（実行計画によっては、n+1になる場合もあるので注意。references使ってる場合はjoinされる）
ModelA.all.each do |m|
m.model_bs # ここで毎回クエリが発行される
end
ModelA.includes(:model_bs).all.each do |a|
a.model_bs
end
2-1-2. ループする必要がないならループしない
※ update_allはコールバックが実行されない点は注意
model_as = ModelA.where(id: [1,2,3,4,5])
model_as.each do |a|
a.model_bs.update(some_flg: true)
end
model_as = ModelA.where(id: [1,2,3,4,5])
model_bs = ModelB.where(id: model_as.map(&:model_b_id))
model_bs.update_all(some_flg: true)
2-1-3. joinしたかったら、先に1000件とか一括で持ってきて、Hashにしとく
model_as = ModelA.where(id: [1,2,3,4,5])
model_as.each do |a|
b = a.model_bs
end
model_as = ModelA.where(id: (1...1000).to_a)
model_bs = ModelB.where(id: model_as.map(&:model_b_id))
model_bs_hash = model_bs.inject({}) do |hash, b|
hash[b.id] = b
hash
end
model_as.each do |a|
b = model_bs_hash[a.model_b_id]
end
2-2. レコード数が多いテーブルはジョインしない
1:nの関係にあったとき、1レコード平均10件の関連レコードがあると、100万レコードのテーブルと関連レコードをjoinしたときに生成される一時テーブルは1000万レコードになります。
1:1の関連の場合など、問題がないケースもありますが、レコード数が多いテーブルを認識し注意を払うことは大切です。
解決方法
だいたいN+1と同じ方法で解決できます
2-3. json, text型を含むテーブルはselect句で必要なカラムに絞って取得する
レコード数が少なくとも、json、text型を含むレコードは1レコードのデータ量が大きくなりがちです。特に1000文字以上の文字列が入ってる場合は注意が必要です。
データ量が多いと、APサーバーとDB間のネットワークやメモリを余分に消費してしまい遅延やエラーの原因になります。
解決方法
2-3-1. select句で取得カラムを絞る
ModelA.all
ModelA.all.select(:column_1, :column_2)
2-3-2. includesしない
N+1と同様です
2-4 逐次で計算させない
毎回DBのmax関数などで計算させながら表示してるなどしてると、毎回負荷がかかる上に、その計算値を使ったsortをしたい場合は毎回全件取得する必要が出てきます。
2-4-1. どこかのカラムやキャッシュに計算結果を保持する
ModelA.select("max(model_bs.id) as max_b_id").
joins(:model_bs).
sort {|a| a.max_b_id}.
first(10)
class ModelA < ActiveRecord::Base
has_one model_a_stat
end
class ModelAStat < ActiveRecord::Base
belongs_to model_a
end
ModelA.select("max_b_id").
joins(:model_a_stat).
order(max_b_id: :asc).
limit(10)
3. セキュリティについて
セキュリティも奥が深いので、全てを網羅できているわけでは全然ないですが、コードを書く上で最低限抑えておいてほしいところを列挙しました。
3-1. paramsの取り扱い
paramsはブラウザをちょっといじったり、curlなどを使えば簡単に操作できてしまうので、自分が期待してる値でないものがくる可能性を常に考えておく必要があります。
3-1-1 params[:user_id]は使わない
current_userなどのsessionから引っ張ってきたインスタンスがapplication_controllerのbefore_filterあたりにだいたいあるので、理由がない限りそちらを使いましょう
ModelA.where(user_id: params[:user_id])
def current_user
User.where(id: session[:id]).first
end
ModelA.where(user_id: current_user.id)
3-1-2 sessionから引っ張ってこれない、params[:xxx_id]
current_userやcurrent_companyなどのsessionから引っ張ってきたインスタンスでの検証も行いましょう
ModelA.where(xxx_id: params[:xxx_id])
ModelA.where(xxx_id: params[:xxx_id], user_id: current_user.id)
3-1-3 プレースホルダーを使う
where句などにparamsを入れる場合はプレースホルダーを使いましょう。
paramsじゃなくても、常にプレースホルダー使えば考えることは少なくなります。
ModelA.where("xxx_id = #{params[:xxx_id]}")
ModelA.where("xxx_id = ?", params[:xxx_id])
3-1-4 xxx_typeなどのenumてきなパラメータや、orderのasc, descなどの定型文字が入ることを期待するパラメータ
case文などを使って、規定文字以外の場合は、何かに倒すか、エラーにしましょう。
ModelA.order(xxx_id: params[:direction])
direction = (params[:direction] == "desc") ? "desc" : "asc"
ModelA.order(xxx_id: direction)
3-2. DB以外の共有リソースの取り扱い
DB以外の共有リソース
- ファイル
- キャッシュ
- S3
- ディレクトリ
- グローバル変数
- クラス変数
- インスタンス変数
問題
DBと違って、transactionなど、エラー時に巻き戻してくれる仕組みなどは基本的にないです。
よって
- 同時アクセスで書き換えられる
- エラーが起きると残ってしまって、次来た処理で不具合が起きる
などが起きえます。
解決方法
3-2-1. 一時的にしか使わないならTmpfile使う
file_path = "some_file_name"
File.open(file_path, "w")
Tmpfile.open
3-2-2. ensureでdeleteやcloseする（Tmpfile使えばcloseで削除されるので不要）
file_path = "some_file_name"
File.open(file_path, "w") do |f|
f.puts "xxx"
end
file_path = "some_file_name"
begin
file = File.open(file_path, "w") do |f|
f.puts "xxx"
end
ensure
File.delete file_path
end
3-2-3. ファイルは追記モード（a）では使わない
file_path = "some_file_name"
File.open(file_path, "a") do |f|
f.puts "xxx"
end
file_path = "some_file_name"
File.open(file_path, "w") do |f|
f.puts "xxx"
end
最後に
改めてこの資料をつくったことで、性能やパフォーマンスの意識が少し上がった気がしています。
今後も常に心がけないとすぐに劣化していってしまうため、定期的に啓蒙活動を続けていく所存です。
利用しているお客様のためにも、抑えるべき部分はしっかり抑えてよりよいプロダクト開発を行っていきます。
また、今後こちらをベースにオンボーディング用資料として充実させていこうと思います。
「もっとこうしたほうがいい」「うちではこうしてる」などあればぜひコメントいただければ幸いです。