reduce ecs api call in running_essential_task? method #37
Conversation
lib/ecs_deploy/auto_scaler.rb
Outdated
| task_groups = service_config.client.describe_tasks(cluster: service_config.cluster, tasks: task_arns).tasks.map(&:group) | ||
| task_groups.include?("service:#{service_config.name}") | ||
| container_arns = service_config.client.list_container_instances(cluster: service_config.cluster, filter: "task:group == service:#{service_config.name}")[0] | ||
| container_arns.include?(instance.container_instance_arn) |
There was a problem hiding this comment.
I confirmed that this diff return the same results.
The test environment like this:
- 1 cluster
- 1 autoscaling group for the cluster
- 2 services in the cluster
- 1 service is specified with
services:namekey in YAML config file for autoscaler- then I stop and run this specified service's task and check result
detail for test environment: https://gist.github.com/woshidan/5463dfeba59cb506c3ca2cc4d2106e4a
lib/ecs_deploy/auto_scaler.rb
Outdated
| task_arns = service_config.client.list_tasks(cluster: service_config.cluster, container_instance: instance.container_instance_arn).task_arns | ||
| task_groups = service_config.client.describe_tasks(cluster: service_config.cluster, tasks: task_arns).tasks.map(&:group) | ||
| task_groups.include?("service:#{service_config.name}") | ||
| container_arns = service_config.client.list_container_instances(cluster: service_config.cluster, filter: "task:group == service:#{service_config.name}")[0] |
There was a problem hiding this comment.
このAPI呼び出しをこのメソッドから分離できる。
これを分離して339行目のループの外側で、serviceに関連するコンテナインスタンスを事前に取得して、このメソッドでは引数として存在するinstanceのarnと比較するだけにすれば、API呼び出し回数は50分の1ぐらいになる。
多分メソッドの置き場所としては242行目にあるServiceConfig#fetch_container_instancesですね。
まず、このメソッドの名前をfetch_all_container_instancesに変える。
でServiceConfig#fetch_container_instancesでサービスが起動したタスクをホストしているcontainer_instancesを取れる様にする。
そうすれば割と自然になりますね。
There was a problem hiding this comment.
検証に利用したecs_autoscalerの設定
polling_interval: 30
auto_scaling_groups:
- name: ag_woshidan_test
region: ap-northeast-1
buffer: 1 # タスク数に対する余剰のインスタンス数
services:
- name: woshidan-test-service
cluster: woshidan-test-cluster
region: ap-northeast-1
auto_scaling_group_name: ag_woshidan_test
step: 3
idle_time: 120
max_task_count: [15]
cooldown_time_for_reach_max: 600
min_task_count: 3
upscale_triggers:
- alarm_name: "TEST ALARM TO TRIGGER UPSCALE"
state: ALARM
downscale_triggers:
- alarm_name: "TEST ALARM TO TRIGGER DOWNSCALE"
state: ALARM
step: 6検証内容・結果
アラートを
| 時刻 | アラートの状態変化 |
|---|---|
| 16:39 | TEST ALARM TO TRIGGER UPSCALE: OK => ARARM |
| 16:51 | TEST ALARM TO TRIGGER UPSCALE: ARARM => OK |
| 16:51 | TEST ALARM TO TRIGGER DOWNSCALE: OK => ARARM |
のように変化させて、
- ecs_autoscalerで管理しているサービス(
woshidan-test-service)のタスクの数 - 同じクラスタのコンテナインスタンスに乗っているecs_autoscalerで管理していないサービス(
woshidan-test-service-2)のタスクの数 - AutoScalingグループのインスタンスのデタッチ、アタッチの履歴
は以下なのでよさそうです(環境構築の詳細は https://gist.github.com/woshidan/2235b3e6a194bd37d795379855d3f2be)。
woshidan
force-pushed
the
reduce-ecs-api-call-in-running-essential-task-method
branch
from
0d2ca6e
to
1da8ccc
Compare
… and fetch the list out of the check if conatiner instance in cluster is deregisterable
woshidan
force-pushed
the
reduce-ecs-api-call-in-running-essential-task-method
branch
from
1da8ccc
to
e6ccd81
Compare
lib/ecs_deploy/auto_scaler.rb
Outdated
| break unless resp.next_token | ||
| end | ||
|
|
||
| chunk_size = 50 |
There was a problem hiding this comment.
これ、arnしか使ってないので、ここから下必要無くて、arnだけ返せば良いと思う。
他の部分は良さそうですね！
There was a problem hiding this comment.
コメントありがとうございます！
そこはたしかにARNしかつかってないんですが、fetch_container_instances_in_service と fetch_container_instances_in_cluster で対照的なメソッドがあるとき、その両方で扱ってるオブジェクトのクラスが同じ方がわかりやすいかもなーってちょっと迷ってます。
直しておいたほうがいいでしょうか。
There was a problem hiding this comment.
そもそもAPIリクエスト回数を減らすのが目的だし、そこが対照的であったとしても、そんなに分かり易さに寄与しないと私は思います。
まあ、これで減るのはserviceの数 * せいぜい1回か2回分ぐらいですが。
後、実質的に使ってないコードがあるより、無い方が良い。
使ってなさそうなコードが残ってると後になって読んだ時にそうである意図に悩むことになる。
…ist because container instances in service is not used in running_essential_task?
|
アラートを
のように変化させて、 #37 (comment) と同様の検証を行ったところ、動作確認の結果はよさそうです 🙆
|
|
ということでマージします！ |
woshidan
deleted the
reduce-ecs-api-call-in-running-essential-task-method
branch
Modify instance deregister logic to reduce ECS API calls.