プログラミング学習中の人が稼働中のシステムに不具合を発生させた話

これは、フィヨルドブートキャンプ Advent Calendar 2020(Part 1) 12日目の記事です。昨日は、ksm さんの プログラミング学習サービスに参加して100日経過した話 - improve.design でした。 フィヨルドブートキャンプ Advent Calender 2020(Part 2)もあります。

アドベントカレンダーというのは、12月1日からクリスマスまで、特定のテーマに沿った記事を公開していくという企画です)

参考:アドベントカレンダー - Wikipedia

先日、私の書いたコードが原因でフィヨルドブートキャンプの学習システムに不具合を起こしてしまいました。そのときの状況や学んだことについて書きます。

読んでもらいたい人

  • フィヨルドブートキャンプで勉強中の人、関係者の皆さん
  • フィヨルドブートキャンプに参加するか迷っている人
  • フィヨルドブートキャンプではどんなことをやっているか知りたい人
  • 読みたい人!

フィヨルドブートキャンプのチーム開発

私は、2019年の12月から約1年間、FJORD BOOT CAMP(フィヨルドブートキャンプ) というプログラミングスクールで勉強しています。カリキュラムの終盤に「チーム開発」があり、それまで自分がずっと使ってきたフィヨルドブートキャンプの eラーニングシステムを開発することになります。

毎週、メンター&チーム開発参加者で振り返りミーティングと計画ミーティングを行い、成果物のデモ、Issue の割り当て、プランニングポーカーなどを行って、20ポイント分マージされたら終了です。

コードはオープンソースなので誰でも見ることができます。

GitHub - fjordllc/bootcamp: プログラマー向けEラーニングシステム

不具合の内容と対応

2週間ほど前に、このチーム開発での私の PR が原因で本番環境で不具合を起こしてしまいました。

アサインされたのはこの Issue。

自分の日報・提出物・イベントをWatchすると通知が二重になる · Issue #2013 · fjordllc/bootcamp

クリック

Issue の説明を書いたら長くなってしまったので、気になる方だけここをクリックして読んでください。

フィヨルドブートキャンプ参加者は、日々ブートキャンプアプリ上で日報や提出物を提出することになっているのですが、それにコメントがつくと、アプリ上で通知されるようになっています。これまでは、日報・提出物の作成者とコメントした人とが異なる場合に、日報・提出物作成者に通知されるようになっていました。

それとは別で、このアプリには「Watch」という機能があります。誰が作成したかにかかわらず、日報や提出物のコメント通知を受け取れる機能です。人の日報などにコメントすると自動で「Watch中」になり、その後のコメント通知が受け取れるようになります。

便利なのですが、自分の日報などにコメントがきて返信したりすると、自分の日報・提出物が「Watch中」になります。つまり、その後の通知は二重になってしまうという問題が発生していました(アプリ上での通知は1件にまとめられるが、メール通知は複数くる)。

ということで、コメント通知を「Watch」だけにして、これまでの「日報・提出物の作成者とコメントした人とが異なる場合に、作成者に通知」はしないようにしたいというIssue が立てられました。

(イベントも含まれますが、今のところイベントを作成した受講生がいないので記載を省略しているところがあります)


私が作成した PR。

日報・提出物・イベントのコメント通知をWatchに変更 by masuyama13 · Pull Request #2104 · fjordllc/bootcamp

この PR でやりたいことは2つありました。

  • ユーザーが日報・提出物・イベントを作成したら、自動で「Watch中」にすること(問題なし)
  • 既存の日報・提出物・イベントについて、作成者自身が「Watch中」となるようにすること(問題発生!)

問題のコード

既存の全ての日報・提出物・イベントの1件1件について、作成者自身のWatchデータを検索、なければ作成するRakeタスクです。ローカルでは問題なく動き、コードとして間違いというわけではありませんが、データベースへのアクセスが多すぎて時間がかかり処理に失敗するという結果になりました。

ActiveRecord::Base.transaction do
  Report.includes(:user, :watches).each do |report|
    Watch.find_or_create_by!(user: report.user, watchable: report)
  end
  Product.includes(:user, :watches).each do |product|
    Watch.find_or_create_by!(user: product.user, watchable: product)
  end
  Event.includes(:user, :watches).each do |event|
    Watch.find_or_create_by!(user: event.user, watchable: event)
  end
end

コードを書いた後に、全部eachで回すと結構重いのかも?という疑問が一瞬頭をよぎりましたが、 1回きりの処理だし全部で数千件ぐらいだからまあ大丈夫かな、includes を使えばOKだろうぐらいに深く考えませんでした(実際には日報だけで2万件以上ありました)。

結果として、既存の日報などのうち作成者本人がWatch中でないものにコメントがついても、通知されないという不具合が発生しました。

対応

1日目

BULK INSERT を試してみてとアドバイスいただいたので、まずそこから調べることに。言葉を聞いたことがある程度で、具体的なやり方など何も知らない状態でした。

調べた結果、insert_allというメソッドが使えそうだと思いました。

ActiveRecord::Persistence::ClassMethods

ローカル環境でいろいろ動かしてみて、全部の日報などを各作成者が「Watch中」にすることはできました。しかし、「そのデータが既にあれば作らず、ない場合だけ作る」という条件分岐的なところがわからず詰まりました。いくつか解決策案を考えてみたものの、自分の実力では実現可能か調べるだけでかなりの時間が必要な中で、方向性が全くわからず不安になりました。

N+1 問題に関しては、これまでBullet(N+1を検知するGem)頼みだったため、コードから読み取る力が自分にはありませんでした。SQL入門書を読み直したり情報収集したりしましたが、この日は結局わかりませんでした。

2日目

これ以上時間をかけても自分の力だけで解決するのは難しいと思い、フィヨルドブートキャンプの Slack の wakaranチャンネルで状況を流しながら作業を進めることにしました。

wakaran チャンネル
わからないことを雑につぶやくチャンネル。Macの使い方から自作サービスの設計の迷いポイントでもなんでもOK!初歩的な質問大歓迎です!

wakaran チャンネルは、「質問するまでのハードルが高い」という受講生の声を受けて数ヶ月前にできたチャンネルで、質問にならない「wakaran」を気軽につぶやけるすばらしいチャンネルです!

状況を書き込むと、すぐにアドバイザーの @udzura さんがアドバイスくださいました。問題を分割することや、やりたいことをまず日本語できちんと整理することなど、問題に対処するときに大事なことをたくさん教えていただきました。

おかげで少しずつ進むことができたのですが、夜中まで頑張っても答えまでは出せませんでした。

3日目

次の日起きたら、今度はメンターの @jnchito さんからアドバイスが届いていました。具体的なコードだけでなく、メンタル的な部分(励ましの言葉など)、実際の現場での対応方法、コードもバルクインサートを使わないバージョン・使うバージョンなどものすごい情報量でした!(フィヨルドブートキャンプ生なら私の日報のリンクから見られます)

数時間後、なんとか PR ができ(ほとんど伊藤さんのコードそのままですが)、マージ。不具合解消となりました。

作成したPR。

既存の日報・提出物・イベントについて、作成者自身をWatch中にする by masuyama13 · Pull Request #2163 · fjordllc/bootcamp

ポイントとしては、テストを書いたことです。伊藤さんから、「できればデータ移行のロジックはモデルに移して、テストを書くことをオススメします。」とアドバイスいただいたのでやってみたのですが、実際テストがあると安心感が全然違いました。

今回の問題はテストがあれば防げたということではありませんが、1回きりの処理だからといってテストを書かなくていい理由にはならないので、やってみてよかったです。

学んだこと(技術面)

バルクインサートとは

今回のように DB に対し何万回もアクセスすると、パフォーマンスが悪くなり処理に失敗することもある。 そこで、大量のデータを1回の命令(問い合わせ/クエリ)でINSERTするのが BULK INSERT。

作業の過程で学んだことメモ

Report.joins(:watches).to_sql
=> "SELECT \"reports\".* FROM \"reports\" INNER JOIN \"watches\" ON 
\"watches\".\"watchable_type\" = 'Report' AND \"watches\".\"watchable_id\" 
= \"reports\".\"id\""

再発防止策案

  • 今回のPRではやりたいことが2つあった。ロジック変更とデータ移行をそれぞれ別のPRにして、データ移行を先行させる
  • 不具合に気づいた時点で一旦 revert する

今回のことを受けて、不具合発生時の対応についてドキュメントが作成されました。不具合は発生しないのが一番ですが、何かあったときにフローがあれば少しは落ち着いて行動できそうです。

学んだこと(技術以外の面)

Working Out Loud(大声作業)の大切さ

前にも一度書いたことがある Working Out Loud。フィヨルドブートキャンプの Slack で紹介されていて知ったものだったと思います。

masuyama13.hatenablog.com

  • 作業が途中であってもチームメンバーの目の触れる場所にガンガンアウトプットする
    • Slackなどのチャットツールで実況中継しながら作業したり
    • 設計やコーディングがWork in progressな状態でもフィードバックを募ったり
  • 作業で詰まったらとにかく尋ねる
    • 簡単に手伝ってもらえるのに1人で行き詰まっているというのはプロの振る舞いではない

Working Out Loud 大声作業(しなさい)、チームメンバー同士でのトレーニング文化の醸成 - Quipper Product Team Blog

他業種から転職を目指す私のような人間から見ると、信じられないような考え方かもしれません。それまでの人生経験によるところが大きそうですが、記事を読んだだけで簡単に実践できるようなものでもないと思います。

でも、エンジニアとして仕事をするなら大事なスキル。私も、最初知ったときはなかなかできませんでしたが、フィヨルドブートキャンプという心理的安全性の高い環境に長くいたおかげで、だんだんできるようになってきました。

今回、自分だけの力では無理だと思ったとき、すぐ wakaran チャンネルで作業した点はよかったと思います。毎日オンラインで行われている「質問・雑談タイム」(メンターのプロのエンジニアの方に質問できる時間)にも参加し、たくさんの方に助けていただきました。

技術的な面だけでなく、「実際の現場でもよくある話だから焦らず落ち着いて対応しましょう」といったアドバイスもいただき、ネガティブにならずに問題の解決に集中することができました。「一人でやってる感」が全くなかったです。

フィヨルドブートキャンプの学習システムでも「分報」機能が実装される予定なので、トラブルになる前から Working Out Loud したらもっとよさそうです。

自分の手がける問題について、「聞きまくれる相手」がいる、というのはスキルの一部だ。
(イシューからはじめよ)

ある意味、フィヨルドブートキャンプはお金を払うだけで「スキル」が手に入るスクールですね(怪しい響きですが、アフィリエイトは一切やっておりません)。

もし FJORD BOOT CAMP(フィヨルドブートキャンプ) に興味を持った方がいたら、冒頭にもはった ksm さんの昨日の記事がいい点・イマイチな点など詳しく書かれていて参考になると思います。まだの方はぜひ読んでみてください〜

そして輪読会開催へ

あらためてデータベースについて学ぶことの必要性を痛感したので、今月末から 達人に学ぶDB設計 徹底指南書 の輪読会を始めることになりました。フィヨルドブートキャンプの参考書籍でもあり、以前さらっとは読んだのですが、難しすぎてほとんど記憶にない…。

第1章は「データベースを制する者はシステムを制す」

そういうことらしいです。システムを制したい人、一緒にやりましょう!