デバイスを落とせるようにする #319
デバイスを落とせるようにする #319
Conversation
5363bc4 to
e633b52
Compare
2274999 to
afea4f1
Compare
Cargo.toml
Outdated
| @@ -41,7 +41,7 @@ raftlog = "0.6" | |||
| rustracing = "0.1" | |||
| rustracing_jaeger = "0.1" | |||
| siphasher = "0.2" | |||
| slog = "2" | |||
| slog = { version = "2", features = ["max_level_debug"] } | |||
There was a problem hiding this comment.
今回の変更で max_level_debug はなぜ必要なんでしょうか。
There was a problem hiding this comment.
実験用にやっていたのが混入してしまいました。削除します
5857183 to
1cae6db
Compare
src/service.rs
Outdated
| false | ||
| } | ||
| /// 単にデバイスのデータを取得するだけなら device_seqno は不要だが、ログ出力のために受け取っておく。 | ||
| pub fn get_device_state(&mut self, device_seqno: u32, device_id: &DeviceId) -> bool { |
There was a problem hiding this comment.
get_device_state の返り値が bool というのは意味が直感的に理解しづらいように思います。DeviceState という struct が追加されているのでそれを返せばいいんじゃないでしょうか。
There was a problem hiding this comment.
DeviceState を返すようにしました 50c2c72
src/daemon.rs
Outdated
| device_id: DeviceId, | ||
| ) -> impl Future<Item = bool, Error = Error> { | ||
| let (reply_tx, reply_rx) = oneshot::monitor(); | ||
| // TODO |
There was a problem hiding this comment.
何が TODO なのか分からないのでそれも書いておいていただけると助かります。
There was a problem hiding this comment.
この TODO は消し残しだったので消しました 6d7ca2e
it/scripts/put_devices.sh
Outdated
| do | ||
| for device in `seq 1 $DEVICES_PER_SERVER` | ||
| do | ||
| for server in $SERVERS; do |
There was a problem hiding this comment.
最後の行に set +x を追加して、 put_devices.sh による余計な行の出力を suppress したかったからでした。(フォーマットが変わっているのはエディタの仕様で勝手にフォーマットされたためです。)
ただ、よく調べてみたら別に必要ではなかったので、このファイルの変更は revert しようと思います。
There was a problem hiding this comment.
なるほど。set +x を追加するにしても他の行は変更せずにそれだけ追加するのがいいんじゃないかと思います。
There was a problem hiding this comment.
revert しました f7cbb4b
src/service.rs
Outdated
| @@ -128,6 +128,41 @@ where | |||
| pub fn stop(&mut self) { | |||
| self.frugalos_segment_service.stop(); | |||
| } | |||
| /// 単に止めるためだけなら device_seqno は不要だが、ログ出力のために受け取っておく。 | |||
There was a problem hiding this comment.
/// には description を書いた方がよくて、コードに対するコメントであれば // で十分じゃないでしょうか。
src/daemon.rs
Outdated
| reply: reply_tx, | ||
| }; | ||
| let _ = self.command_tx.send(command); | ||
| StopDevice(reply_rx) |
There was a problem hiding this comment.
get_device_state が StopDevice という struct を返すのは妙な感じがします。別の struct を定義するか、単純に MonitorError を map_err するだけで十分じゃないでしょうか。
There was a problem hiding this comment.
コピペの変更漏れだと思います。修正します
There was a problem hiding this comment.
修正しました (reply_rx を単に map_err しました) 6d7ca2e
b47c26d to
6f0f3ac
Compare
6f0f3ac to
f7cbb4b
Compare
Types of changes
Please check one of the following:
masteranddevelop!)Description of changes
Behavior
/v1/devices/:device_id/stateに以下のような JSON を PUT することで、指定されたデバイス ID を持つデバイスを落とせるようにする。制約は以下の通り:
また、
GET /v1/devices/:device_id/stateが、指定されたデバイス ID を持つデバイスが現在動作しているかを返すようにする。レスポンスは以下のうちどちらか:
Purpose
デバイスを外部から手動で落とせるようにする。
リクエストの流れ
デバイス停止・デバイスの情報取得リクエストの処理の流れ:
未解決事項
デバイスを落とす時に MDS node が CRIT エラーを出して止まる。これは後の PR で対応する予定。
依存 PR: frugalos/cannyls#48
How to test this PR
scripts/setup_debug_cluster.shで起動できるローカルクラスタを起動し、以下を実行する:この状態では (まだデバイスを停止していないため)
{"state":"started"}が返る。その後、以下を実行する:その後、以下を実行すると、
{"state":"stopped"}が返る。注記事項
cargo のバージョンアップに伴って
Cargo.lockのフォーマットが変わっている。おそらく rust-lang/cargo#7579 の変更が入ったことによるものである。本来別の PR として分けるべきであるが、それによって起こるであろうCargo.lockのコンフリクトを解消する手間を避けるためにこのままにする。TODOs
Checklists
cargo fmt --all.cargo clippy --all --all-targets.