Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add autoware_node_death_monitor package for monitoring node crashes #1786

Draft
wants to merge 4 commits into
base: tier4/main
Choose a base branch
from

Conversation

kyoichi-sugahara
Copy link

@kyoichi-sugahara kyoichi-sugahara commented Feb 7, 2025

Description

Draft PR mainly for reviewing changes

以下READMEに追記しますが、一旦ここにメモらせてください。→ b0dae02 で追記

  • MRM を publish していないですが、l4_toolkit に実装する際には MRM の publish を追加する予定なのでそこはスルーしていただけると 🙇
  • この node が動いていることを保証するのはこの node から heartbeat topic を publish して、 topic_state_monitor から監視する想定です。

Related links

Parent Issue:

  • Link

How was this PR tested?

psim で以下の動作確認を実施済み

  • rtc_interface を kill すると MRM で停止できないが、このノードで検知できることを確認
  • rviz を kill しても ignore できることを確認

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

Copy link

@TakaHoribe TakaHoribe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

実装の細かい部分は他の方に任せますが、ハイレベルな設計で気になったポイントを書きます。

・launch.logって必ず書き込まれるんだっけ?その保証をしておきたい。
・これってノードが死んだらDiagでエラーだす実装を追加する、で良いですよね?
・ファイルが見つからなかったときに、初回でエラーが出るだけなので「node_death_monitorが立ち上がっているからOKだと思っていたらファイル見れてなかった、実はノード死んでた」ケースがありそう。起動して10秒経ってもファイル見つからない場合は、それ自体でDiagエラー出した方が良い。

それ以外、全般的な方針は問題/違和感ありません。

cmake_minimum_required(VERSION 3.14)
project(autoware_node_death_monitor)

find_package(autoware_cmake REQUIRED)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about node_alive_monitor or process_alive_monitor?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed name to process_alive_monitor in 572427d

@@ -0,0 +1,73 @@
# autoware_node_death_monitor

This package provides a monitoring node that detects ROS 2 node crashes by analyzing `launch.log` files, rather than subscribing to `/rosout` logs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

launch.logにメッセージが出力されることって保証されてましたっけ?もしyesならREADMEに「ROS2の仕組みとして必ずlounch.logに出力される」とか、もしくは「ooオプションがONになっていることを確認すること」とかを記載したい。

std::streampos last_valid_pos = static_cast<std::streampos>(last_file_pos_);

size_t iteration = 0;
while (true) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while(true)はNG, while(rclcpp::ok)みたいなの無かったっけ。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed in 572427d

@kyoichi-sugahara
Copy link
Author

@TakaHoribe

launch.logって必ず書き込まれるんだっけ?その保証をしておきたい。

書き込まれることは保証されている認識ですが、どこに書き込まれるかは環境設定に依存しているので運用側とのコミュニケーションが必要と思っている点です。
平松さんが letterbox で同じことをやっているので、上記の疑問点はクリアにしておきます。

これってノードが死んだらDiagでエラーだす実装を追加する、で良いですよね?

です。

ファイルが見つからなかったときに、初回でエラーが出るだけなので「node_death_monitorが立ち上がっているからOKだと思っていたらファイル見れてなかった、実はノード死んでた」ケースがありそう。起動して10秒経ってもファイル見つからない場合は、それ自体でDiagエラー出した方が良い。

これは heartbeat topic を出して topic_state_monitor から確認する予定です。(異常系開発の議事録側には書いてたのですが、こちらに書いてないことで誤解招いてしまっているので、READMEに追記します 🙇

@Naophis
Copy link

Naophis commented Feb 12, 2025

@kyoichi-sugahara
2点コメントです。

  1. timer callbackで疑似tail実装よりも、inotifyのほうがkernelレベルで待機してくれるのでリソース的に安上がりかと。
  2. node死をnodeで監視するというのが、アーキテクチャ的に良いのか?というのあります。極端ですがtimercallbackが止まると最悪全部止まるわけなので。(折衷案があるので少々お待ちを・・・)

@kyoichi-sugahara
Copy link
Author

@Naophis
コメントありがとうございます!素敵です ❤️

node死をnodeで監視するというのが、アーキテクチャ的に良いのか?というのあります。極端ですがtimercallbackが止まると最悪全部止まるわけなので。(折衷案があるので少々お待ちを・・・)

ありがとうございます!折衷案の共有大変たすかります。
ただ結局のところ /diagnostic で出力し、別ノードがその情報を受けとり MRM に移行するのでノード死検知にノードを使うことは問題に感じてないです。
この node の heartbeat を監視するので、ノードが正常に動いていなければ検知できる状態になります。
むしろ、 MRM に関連するノード側を正常に動作することをどのように保証するか、の方が懸念かなーという感覚です。

@Naophis
Copy link

Naophis commented Feb 12, 2025

@kyoichi-sugahara
折衷案の図(たたき台)です。

中間にlaunch.logを使った死活監視がいて、ROSとの界面を担うやつが標準入力を受けて鳴く という構成です。
死活監視は、ROSの世界から独立しており、syslogへの書き込み役も兼ねているため、ROSの都合に依る見逃しがない構成としてこのようなことを考えました。

image

この構成のメリットは、ECU内でautowareを分割起動する構成の場合でも、publishするnodeは1つで監視する非ROSの軽量なサービスを複数用意するだけで実現できる点です。

標準入力がセキュリティ的にもあまり受け入れられないかもな〜とか思ってたりもしますので、他のスマートな方法があれば是非という感じでした(ここが自分の中で悶々と悩んで作れなかったところです)
追記:UDSか 名前付きパイプあたりが落とし所?(リッチにやるなら共有メモリですけど、そこまでやるか?という感じです)

@kyoichi-sugahara
Copy link
Author

kyoichi-sugahara commented Feb 14, 2025

@Naophis
すみません、遅れました 🙇
アーキテクチャとしては良さそうに感じます!!!
ただ、今回やりたいこととと時期感 → 特定自動運行向けにx2: v4.3.0 でノード死を監視できるようにする
を踏まえると今回の PR の変更を適用するで十分、かつコスパが良いのではという感覚でいます。
上記の観点で違和感あればコメントいただきたいです。

ちなみに細かい点で気になったのは call_diag で対象外のノードなどを指定する構造になっているかとおもいますが、ノードの指定は autoware_launch ではなく別リポジトリのyamlファイル等で指定する想定でしょうか?

@Naophis
Copy link

Naophis commented Feb 14, 2025

@kyoichi-sugahara
実現手法としては確かに安上がりですが、「監視役が身内」という構造が怖いと思う次第です。

このnodeってどのように起動し待機することを想定してますか?
パッと見、mainでlaunchすることを前提で動くことを想定しているように見えますが、

  1. autoware_launchと一緒に起動?
  2. autoware_launch前に起動?

どちらになります?

@kyoichi-sugahara
Copy link
Author

@Naophis

実現手法としては確かに安上がりですが、「監視役が身内」という構造が怖いと思う次第です。

アーキテクチャーのべき論でいえば、理解はできるのですがどれくらい怖いのかあまりイメージできていないです。
前述の通りこのノードが動作していない場合は MRM で停止する想定なので、それでカバーできるという感覚です。
たぶん実課題に関する理解が不足しているためだと思うのですが、具体的に実運用上問題になるケースが想定されますでしょうか?

autoware_launchと一緒に起動?
autoware_launch前に起動?

autoware_launch と一緒に起動です!

@Naophis
Copy link

Naophis commented Feb 14, 2025

@kyoichi-sugahara

どれくらい怖いのかあまりイメージできていないです。

問題が起きたとき、活躍してほしい時に限って道連れになるので、独立したほうが良いと思ってのコメントです。(ココらへんは理想形に対し近づき方・歩幅の問題かも?)

具体的に実運用上問題になるケース

現在の手法だと、

  • プロダクトによってはautoware自体を分割起動するようになるので、Main ECUで一度にまとめて起動する必要がある。
  • 他のECUはカバーできない。

が考えられます。
Main ECUだけを重点的にケアする優先順位であるなら、現状の仕様でも問題ありませんが、特定自動運行に向けとなると、他のECUについても同様の話が言えると思います。拡張性の観点から現在の構成は横展開が面倒では?と思ってました。

前述の通りこのノードが動作していない場合は MRM で停止する想定なので、それでカバーできるという感覚です。

この部分については、なるほどです。


あとは、そもそも起動できてないやつって検出出来ないと思うのですが、それってありなんですかね・・・?
(この観点に置いては、私の上げたアーキ図ももちろんダメ。)

本当は、
Node死を知りたい ← 実は正解ではない
ちゃんと起動してないNodeを知りたい ← こっちでは?

ホワイトリストを持つ必要があるのですが、特定自動運行向けならありだと思います。
参考:https://github.com/tier4/autoware.universe/blob/tier4/main/system/autoware_duplicated_node_checker/src/duplicated_node_checker_core.cpp#L55

(pubsub_inspectorも似た方法でサーチしてます)

@kyoichi-sugahara
Copy link
Author

@Naophis

Main ECUだけを重点的にケアする優先順位であるなら、現状の仕様でも問題ありませんが、特定自動運行に向けとなると、他のECUについても同様の話が言えると思います。拡張性の観点から現在の構成は横展開が面倒では?と思ってました。

たしかにですね。ECUごとに立ち上げればよいのではという感覚でいますが、その場合運用面の煩雑さが懸念ですかね。

ちゃんと起動してないNodeを知りたい ← こっちでは?

ですね!ホワイトリストの作成の煩雑さを回避するべく、はじめはすべてのノードが立ち上がるという前提で今回のノード死検知でちゃんと起動していない Node を知りたいの要求を満たすイメージです。
ただこれは topic_state_monitor で監視する topic を増やせばできることで別途 @technolojin さんが取り組んでいる活動で巻き取られます。ただそちらの作業が時間がかかりそうなこと、実現可能性が若干不透明なこともあり、そのバックアッププランとしての側面もあります。

ホワイトリストを持つ必要があるのですが、特定自動運行向けならありだと思います。

並行してget_node_names()を使って未知のノード名であれば追加、リストに含まれているノードが取得できなかったらノードが減っていると判断して diag を出すという実装も取り組んでいたのですが、システムを終了させるたびに diag を出すことになりそうなのでそちらのアプローチはやめました。

あれ、でもよく考えたら topic_state_monitor も同じ問題抱えていますかね?

@kyoichi-sugahara
Copy link
Author

あれ、でもよく考えたら topic_state_monitor も同じ問題抱えていますかね?

栗原さんに確認したところ、自動運転中は MRM に移行しないように autoware_diagnostic_graph_aggregator で設定されるためシステム終了時の検知がされないようになっているかも、ということでした。
実装確認しましたがそうなっていそうです。であれば、

get_node_names()を使って未知のノード名であれば追加、リストに含まれているノードが取得できなかったらノードが減っていると判断して diag を出すという実装も取り組んでいたのですが、システムを終了させるたびに diag を出すことになりそうなのでそちらのアプローチはやめました。

というアプローチ + ブラックリストの方がシンプルで筋が良いように感じてきました。

@Naophis
Copy link

Naophis commented Feb 14, 2025

これはメモ程度の話なのですが

未知のnodeについては、ros2 topic echo/hz/list etc... をCLIで実行するとdiscovery protocolに負荷をかけて、たまに色々やらかすのですが、CLI実行時にもインスタントのnodeのエントリーは行われるので、”知らないNodeリスト"があると、別の用途もあるかもです。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants