[rails/solid_queue] スレッド置換時の未定義変数参照を修正

rails/solid_queue

Context

Solid Queueの非同期スーパーバイザーにおいて、失敗したスレッドを置き換える際に未定義の変数を参照するバグが存在していました。この問題は、スレッドの置換処理中に誤った変数を使用することで、正しいエラーハンドリングが行われない可能性がありました。また、テストの不安定性(flaky tests)も複数存在しており、特にプロセス登録の待機処理においてスケジューラが考慮されていないなど、テストの信頼性に課題がありました。

Technical Detail

変数参照の修正

lib/solid_queue/async_supervisor.rbreplace_threadメソッドにおいて、スレッド置換時のエラー処理で誤った変数が参照されていました。

変更前:

error = Processes::ThreadTerminatedError.new(terminated_instance.name)
release_claimed_jobs_by(terminated_instance, with_error: error)

変更後:

error = Processes::ThreadTerminatedError.new(instance.name)
release_claimed_jobs_by(instance, with_error: error)

この修正により、terminated_instanceという未定義の変数ではなく、正しくinstanceパラメータを使用するようになりました。これにより、スレッド終了時のジョブのクリーンアップが適切に実行されます。

テストの安定性向上

複数のテストファイルで、プロセスの登録待機やジョブの完了待機に関する改善が行われました。

プロセス登録の待機改善

非同期スーパーバイザーのテストでは、スケジューラを含む正確なプロセス数を待機するように修正されました。

supervisor = run_supervisor_as_thread(workers: [], dispatchers: [ { batch_size: 100 } ], skip_recurring: false)
wait_for_registered_processes(3, timeout: 3.seconds) # supervisor + dispatcher + scheduler

assert_registered_processes(kind: "Supervisor(async)")
assert_registered_processes(kind: "Worker", count: 0)
assert_registered_processes(kind: "Dispatcher", supervisor_id: supervisor.process_id)
assert_registered_processes(kind: "Scheduler", supervisor_id: supervisor.process_id)

シャットダウン時の挙動改善

非同期モードでのプロセス終了テストでは、スーパーバイザーとワーカーの両方がshutdown_timeoutを使用するため、終了ステータスの競合状態が発生する可能性が考慮されました。

# Wait for process to terminate. In async mode, shutdown_timeout is used by both
# the supervisor and workers, creating a race: exit status may be 0 (graceful) or
# 1 (exit!) depending on which timeout check happens first.
wait_for_process_termination_with_timeout(@pid, timeout: SolidQueue.shutdown_timeout + 5.seconds, exitstatus: nil)

対応するヘルパーメソッドも修正され、exitstatusnilの場合はステータスチェックをスキップするようになりました。

assert_equal exitstatus, status.exitstatus, "Expected pid #{pid} to exit with status #{exitstatus}" if status.exitstatus && !exitstatus.nil?

タイミング依存の問題解決

ジョブの処理状態を確認する前に、適切な待機処理を挿入することで、タイミングに依存する不具合を解消しました。

# Wait for the "no pause" job to complete and the pause job to be claimed.
# This ensures the pause job is actively being processed.
wait_for_jobs_to_finish_for(3.seconds, except: pause)
wait_for(timeout: 2.seconds) { SolidQueue::ClaimedExecution.exists?(job_id: SolidQueue::Job.find_by(active_job_id: pause.job_id)&.id) }

その他の改善

  • テストクリーンアップ処理にClaimedExecutionFailedExecutionの削除を追加
  • トランザクショナルテストの無効化により、プロセス間でのデータ可視性を確保
  • 定期タスクのテストでタスク登録の明示的な待機を追加
  • Instrumentationテストのスリープ時間を調整し、タイミング問題を軽減

Impact

この修正により、Solid Queueの非同期スーパーバイザーにおけるスレッド置換処理の信頼性が向上しました。また、テストスイート全体の安定性が改善され、CI環境での偽陽性/偽陰性の発生率が減少することが期待されます。特に、プロセスのライフサイクルに関連するテストが、より予測可能で信頼性の高いものになりました。

記事メタデータ

Generated by:
Claude Sonnet 4.5 for DiffDaily

この記事はAIによって自動生成されています。内容の正確性については、必ずソースコードやPRを確認してください。

品質レビュー結果

Review Status:
承認済み
Review Count:
1回
Reviewed by:
Gemini 2.5 Pro for DiffDaily

Review Criteria:

ガイドライン準拠 ✓ PASS

記事構成とDiffDaily Styleへの準拠状況

記事構成の3要素(Title, Context, Technical Detail)が明確に記載されており、カスタムMarkdown構文(コードブロック前後の空行、ファイル名付きハイライト)も正しく使用されています。対象読者であるエンジニアに適した内容です。

  • 記事構成(Title、Context、Technical Detail)
  • DiffDaily Styleガイド準拠
  • カスタムMarkdown活用
  • 対象読者への適合性
技術的整合性 ✓ PASS

技術的な正確性と表現の適切性

引用されているコードスニペットと、それに対する技術的な説明が正確です。「terminated_instance」から「instance」への変数名の修正や、テストコードにおける待機処理の改善など、Diffの内容を正しく反映した解説がなされています。

  • 技術用語の正確性
  • コード例の正確性
  • 説明の技術的正確性
PR内容との整合性 ✓ PASS

元のPR情報との一致度

PRのタイトル「Fix undefined variable when replacing a failed thread」と、それに付随するテストの安定化という内容を正確に反映しています。PR情報に基づかない推測や主張(ハルシネーション)は見られず、事実に基づいた記事となっています。

  • タイトル・説明の一致
  • Diff内容の正確な反映
  • 推測の排除