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

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イベントが発生するたびに（５秒毎）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の領域を使ってるのはほとんどないのはなんでだろう？それはそれでまずくないか。。。

さてどうしよう

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

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

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