Added ability to poll for container stats instead of constant stream #1475
Conversation
|
Hi @jcbowman , Thanks, |
0d2a143 to
3205ef6
Compare
|
Thanks for the guidance. I changed the base branch to dev and squashed my changes. For some reason one of the builds started failing after pointing to dev. At a first glance the failing test doesn't appear related to these changes - I'll try to take a deeper look at it soon. |
|
@jcbowman It looks like it was just a transient failure -- so no worries! |
3205ef6 to
3163afa
Compare
|
Is there any ETA for this change to be merged and rolled out? |
agent/config/config.go
Outdated
| @@ -271,6 +283,11 @@ func (cfg *Config) validateAndOverrideBounds() error { | |||
| cfg.TaskMetadataBurstRate = DefaultTaskMetadataBurstRate | |||
| } | |||
|
|
|||
| if cfg.PollingMetricsWaitDuration < minimumPollingMetricsWaitDuration || cfg.PollingMetricsWaitDuration > maximumPollingMetricsWaitDuration { | |||
| seelog.Warnf("Invalid value for polling metrics wait duration, will be overridden with the default value: %s. Parsed value: %v, minimum value: %v, maximum value: %v.", DefaultPollingMetricsWaitDuration.String(), cfg.PollingMetricsWaitDuration, minimumPollingMetricsWaitDuration, maximumPollingMetricsWaitDuration) | |||
There was a problem hiding this comment.
nit: can you split this into multiple lines? Also, this was only required if PollMetrics is true.
There was a problem hiding this comment.
I updated these to only run if PollMetrics is true and split this into two different checks
f7728c7 to
1bd8433
Compare
1bd8433 to
062d013
Compare
|
@richardpen I think I've addressed the PR comments. I squashed my commits to hopefully make merging easier. Let me know if there is anything else I should address. I've also deployed a version with the recent changes to one of our ECS servers - CPU is staying low and stats are showing up in CloudWatch. 👍 |
Oh nice, this is great to hear. @jcbowman, @boynux: We'd like to get this PR moving. Please address the branch conflicts and also unit tests appear to be failing. You can validate unit tests locally with |
|
closing this in favor of #1646, please track the progress of this change through the new PR |
Summary
Allows for container stat polling instead of forcing constant stream. This allows for a significant decrease in CPU usage when a lot of containers are deployed.
Implementation details
There's 2 environment configuration variables that allow you to disable stream (default is to leave streaming turned on) and specify the polling interval (default is 15 seconds).
Testing
This was tested by replacing the ecs-agent container on a ECS host with over 150 containers. We've been using the polling code for the past month in both our prod and nonprod environments.
make release)go build -out amazon-ecs-agent.exe ./agent)make test) passgo test -timeout=25s ./agent/...) passmake run-integ-tests) pass.\scripts\run-integ-tests.ps1) passmake run-functional-tests) pass.\scripts\run-functional-tests.ps1) passNew tests cover the changes:
Description for the changelog
Feature - Allow for Container Stat Polling
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
fixes #1422