[review] delayed_cron_job を入れ替える rake タスク #33
Conversation
SgFargateRails module にいれる
morikiyo
changed the title
| end | ||
|
|
||
| def cron_settings | ||
| @cron_settings ||= Rails.application.config_for('delayed_cron_jobs') |
There was a problem hiding this comment.
delayed_cron_jobs.yamlのテンプレートは作成しない？
上記のテンプレートを作成するかどうかの設定化もやった方がよさそう。
そして、READMEにも追加したいね。
まだ検証フェーズということであればあとでやるでもよさそう。
There was a problem hiding this comment.
テンプレートジェネレートした方が良さそう。
追加しておきます！
There was a problem hiding this comment.
| private | ||
|
|
||
| def destroy_cron_jobs! | ||
| DelayedCronJobUtility.cron_jobs.find_each do |delayed_job| |
There was a problem hiding this comment.
この条件だけだと、現在進行系で動いてるジョブのレコードを消すことにならない？と思いました。（ちょうどいい属性があるかは未確認）
There was a problem hiding this comment.
実行中のジョブもキューデータが消されます。消えてもワーカーとしては問題なく動作することを確認してます。
There was a problem hiding this comment.
懸念があるとすると
処理が長引いて、本来はスキップされるはずの次のスケジュールに新しくジョブが追加されて、別のワーカーが部分的に並列実行しちゃうかも
でも、これは opsworks でも起きてるはずなので（そもそもスケジュールスキップされないはず）問題ではなさそう
There was a problem hiding this comment.
ジョブが成功する場合はいいとして、例外などで失敗した場合も問題ないですか？
ジョブが失敗したことを記録する先がなくなって、期待しない動作にならないかなーと。
There was a problem hiding this comment.
考えてなかった！
処理用の別のジョブを産んでくれればよいのにな delayed_cron_job ...
delayed_cron_job → rake task → delayed_job という処理順にしてもらうしかないのかな
There was a problem hiding this comment.
エラー情報が紛失する？
=> bugsnag に残るはず
リトライを期待してたら困る？
=> 現状 (OpsWorks) の cron はリトライされないからその想定はないはず
There was a problem hiding this comment.
異常系の実機検証してみた。
cron job でエラーが発生したら jobmon, bugsnag, cloudwatch logs にエラー情報が残る
DB (delayed_jobs) からは deploy でエラー情報が消えてしまうが、↑の3つのエラー情報があるから問題はなさそう。
cron job の処理実行中にデプロイによる refresh タスクが実行されたら
コンテナが停止され
jobmon => running のままで通知が来る
bugsnag => "rake aborted!"
cloudwatch logs => SIGTERM のログあり
[Worker(host:ip-10-1-0-74.ap-northeast-1.compute.internal pid:41)] Job Jobmon::TaskJob [030e6b50-6b26-410c-8287-76f61d6bb786] from DelayedJob(default) with arguments: [{"task"=>"cron:long_process_test", "estimate_time"=>1800, "_aj_ruby2_keywords"=>["task", "estimate_time"]}] (id=728) (queue=default) FAILED with SignalException: SIGTERM
E, [2023-12-01T16:39:25.166443 #41] ERROR -- : 2023-12-01T16:39:25+0900: [Worker(host:ip-10-1-0-74.ap-northeast-1.compute.internal pid:41)] Job Jobmon::TaskJob [030e6b50-6b26-410c-8287-76f61d6bb786] from DelayedJob(default) with arguments: [{"task"=>"cron:long_process_test", "estimate_time"=>1800, "_aj_ruby2_keywords"=>["task", "estimate_time"]}] (id=728) (queue=default) FAILED with SignalException: SIGTERM
lib/sg_fargate_rails/railtie.rb
Outdated
| @@ -1,4 +1,5 @@ | |||
| require 'sg_fargate_rails/adjust_cloudfront_headers' | |||
| require 'sg_fargate_rails/delayed_cron_job_utility' | |||
There was a problem hiding this comment.
delayed_jobが有効な場合のみrequireするほうが良さそう。
There was a problem hiding this comment.
ですよね。
defined?(::DelayedCronJob) は、このタイミングではクラスロードの関係か、まだ使えませんでした。
その場合どんな判定処理をするのがよさそうでしょう？
There was a problem hiding this comment.
必要になったタイミングでrequireするほうがエコだし、refresh_delayed_cron_jobsのrakeタスク内でrequireすれば良さそう！
There was a problem hiding this comment.
そうか！ここで require するものと完全に思い込んでました！
| Rails.logger.info('[refresh_delayed_cron_jobs] refresh begin...') | ||
| SgFargateRails::DelayedCronJobUtility.refresh_cron_jobs! | ||
| Rails.logger.info('[refresh_delayed_cron_jobs] refresh end.') | ||
| end |
There was a problem hiding this comment.
delayed_jobが有効な場合のみrakeタスクを定義するほうが良さそう。
参照: #19