[rails/solid_queue] スレッド置換時の未定義変数参照を修正
Context
Solid Queueの非同期スーパーバイザーにおいて、失敗したスレッドを置き換える際に未定義の変数を参照するバグが存在していました。この問題は、スレッドの置換処理中に誤った変数を使用することで、正しいエラーハンドリングが行われない可能性がありました。また、テストの不安定性(flaky tests)も複数存在しており、特にプロセス登録の待機処理においてスケジューラが考慮されていないなど、テストの信頼性に課題がありました。
Technical Detail
変数参照の修正
lib/solid_queue/async_supervisor.rbのreplace_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)
対応するヘルパーメソッドも修正され、exitstatusがnilの場合はステータスチェックをスキップするようになりました。
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) }
その他の改善
- テストクリーンアップ処理に
ClaimedExecutionとFailedExecutionの削除を追加 - トランザクショナルテストの無効化により、プロセス間でのデータ可視性を確保
- 定期タスクのテストでタスク登録の明示的な待機を追加
- Instrumentationテストのスリープ時間を調整し、タイミング問題を軽減
Impact
この修正により、Solid Queueの非同期スーパーバイザーにおけるスレッド置換処理の信頼性が向上しました。また、テストスイート全体の安定性が改善され、CI環境での偽陽性/偽陰性の発生率が減少することが期待されます。特に、プロセスのライフサイクルに関連するテストが、より予測可能で信頼性の高いものになりました。