読者です 読者をやめる 読者になる 読者になる

matsukaz's blog

Agile, node.js, ruby, AWS, cocos2d-xなどなどいろいろやってます

hubot-brainやhubot-cronまわりの問題点整理

hubot Node.js coffeescript JavaScript

hubot-brainやhubot-cronまわりについて調べていたところ、

の記事を発見。ソースコードを確認したら、確かにhubot-brainのmergeDataまわりが実装がヤバイっぽい(;´Д`)
とりあえず把握できてる問題点を整理してみた。

hubot-brainの仕組み

まずhubot-brainについてざっくりまとめると、

  • hubot-brainは、hubotでデータを永続化するための仕組み
  • hubot初期化時のコンストラクタですぐに初期化され、robot.brainでアクセス可能
  • robot.brainは、データ格納用にdataというインスタンス変数を保持
console.log robot.brain.data
    //-> { users: {}, _private: {} } 初期化直後
  • set/get/removeなどのアクセサメソッドを利用することで_private内のデータを操作できる
robot.brain.set 'hoge', 'fuga'
console.log robot.brain.data
    //-> { users: {}, _private: { hoge: 'fuga' } }
  • mergeDataメソッドが呼び出されると、dataの中身が全てその内容で置き換わる
console.log robot.brain.data
    //-> { users: {}, _private: { hoge: 'fuga' } }
robot.brain.mergeData {}
console.log robot.brain.data
    //-> {}
  • setとmergeDataが呼ばれると、loadedイベントがemitされる(データが更新された、という位置付けらしい)
console.log robot.brain.data
    //-> { users: {}, _private: {} }
robot.brain.on 'loaded' =>
  console.log robot.brain.data
      //-> { users: {}, _private: { hoge: 'fuga' } }
robot.brain.set 'hoge', 'fuga'
  • 5秒毎にsaveイベントがemitされる(setAutoSaveとかresetSaveIntervalで間隔やイベント発生の有無を制御できる)

という感じ。

redis-brainの仕組み

んでhubot-brainのデータをRedisに永続化するためのredis-brainは、hubot-brainを以下のように使ってた。

  • まずsetAutoSave(false)を呼び出すことでsaveイベントのemitを止める
  • Redisに接続
  • Redisに接続されたらデータを取得し、mergeDataメソッドを呼び出してrobot.brain.dataの内容を変更
  • setAutoSaveをtrueに戻す
  • robot.brainのsaveイベントをlistenすることで、saveイベントが発生するたびに(5秒毎)Redisにrobot.brain.dataの内容を書き込む

ここまでで問題になるのは

  • Redisにつながったらデータを置き換えるmergeDataメソッドが呼ばれる=Redisにつながる前のデータは全て消える(!)
    • Redisが落ちてて、その後Redisを起動した場合がやばい(起動時もだし、運用中にRedisを再起動した場合も同様)
  • loadedはデータが更新されたことを表すイベントで、mergeDataだけでなくsetでも呼ばれるので、Redisと同期されてる状態かどうかの判断には使えない
  • setAutoSaveやresetSaveIntervalを他スクリプトが操作した時点で、Redisとの同期が保証されなくなる

といった辺り。ぬぅ。

hubot-cronの仕組み

hubot-cronは、hubot上で動作するcronのジョブを登録できるスクリプト
登録したジョブをredis-brainを使って永続化してるけど、loadedイベントの扱い方をまずくて、Redisからのデータ読み込み完了時を想定した実装になっていた。
loadedイベントをlistenしてRedisから読み込んだジョブを再登録をしてるので、loadedイベントが発生するたびにジョブが重複して登録されていくという・・・(;・∀・)

ただ、hubot-cronだけ使ってるとこの問題に気づきにくいかも。
というのも、hubot-cronではアクセサメソッドを介さずに直接robot.brain.dataを操作しているため。

robot.brain.data.cronjob[id] = job.serialize()

これなら確かにloadedイベントが発行されないので大丈夫・・だけど、hubot-cron以外のスクリプトがsetメソッドを呼んだ時点で破綻してる。。。
そもそも運用中にRedisを再起動するだけでジョブが重複登録されちゃってるわ(;´Д`)

$ bin/hubot
Hubot> hubot new job "0 25 12 * * *" メッセージだよ
(この間にRedisを3回停止&起動してみた)
Hubot> メッセージだよ
Hubot> メッセージだよ
Hubot> メッセージだよ
Hubot> メッセージだよ

ちなみに調べた感じだと、hubot-cronみたいにアクセサメソッドを使わずにrobot.brain.dataを直接触ってるスクリプトが多いみたい。
_privateの領域を使ってるのはほとんどないのはなんでだろう?それはそれでまずくないか。。。

さてどうしよう

いろいろ問題があることは分かったので、どう対応しようか考え中ですw



2015/01/13 01:06 追記

普通に困ったので、loaded時に無条件でジョブを登録するのではなく、既存のジョブと差分をチェックして、必要に応じてジョブを追加したりstoreするような修正をしてプルリク投げてみた。

当面はこのプルリクの内容のスクリプトを直接使って対応しようかな。

2015/01/14 14:02 追記

そもそもmergeDataの動きがマージじゃなくて上書きなのが問題だよね、というのを宮川さんとプルリクベースで話せたので、hubot本家の方にmergeDataについてissue投げてみた。
っていうかコメントにも

# Caveats: Deeply nested structures don't merge well.

とかって書いてあったし。誰も直してないだけっぽい。




WEB+DB PRESS vol.82ではhubotの特集とかしてた。

WEB+DB PRESS Vol.82

WEB+DB PRESS Vol.82

  • 作者: 山口徹,Jxck,佐々木大輔,横路隆,加来純一,山本伶,大平武志,米川健一,坂本登史文,若原祥正,和久田龍,平栗遵宜,伊藤直也,佐藤太一,高橋俊幸,海野弘成,五嶋壮晃,佐藤歩,吉村総一郎,橋本翔,舘野祐一,中島聡,渡邊恵太,はまちや2,竹原,河合宜文,WEB+DB PRESS編集部
  • 出版社/メーカー: 技術評論社
  • 発売日: 2014/08/23
  • メディア: 大型本
  • この商品を含むブログ (1件) を見る

ABCロボ アルファボット 26個セットBOX

ABCロボ アルファボット 26個セットBOX