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

Reduce the acs inactivity timeout to 1 minute #912

Merged
merged 1 commit into from
Aug 7, 2017

Conversation

richardpen
Copy link

@richardpen richardpen commented Aug 2, 2017

Summary

This will reduce the acs disconnection period, the maximum disconnection period of acs would be 1 minute before agent try to connect to acs again. This should mitigate the issue #781.

Implementation details

Change the default heartbeat timeout from 5 minute + random(3minutes) to 1 minute.

Testing

  • Builds on Linux (make release)
  • Builds on Windows (go build -out amazon-ecs-agent.exe ./agent)
  • Unit tests on Linux (make test) pass
  • Unit tests on Windows (go test -timeout=25s ./agent/...) pass
  • Integration tests on Linux (make run-integ-tests) pass
  • Integration tests on Windows (.\scripts\run-integ-tests.ps1) pass
  • Functional tests on Linux (make run-functional-tests) pass
  • Functional tests on Windows (.\scripts\run-functional-tests.ps1) pass
  • Run the agent for few hours, check the acs disconnection/connection period.
    New tests cover the changes:

Description for the changelog

  • Enhancement - Reduce the disconnection period of agent to backend.

Licensing

This contribution is under the terms of the Apache 2.0 License:
yes

@vsiddharth
Copy link
Contributor

Could you please check the boxes in the summary and also add an explanation as to why the jitter was removed in the commit message body?

Thanks

@@ -45,8 +45,7 @@ import (
const (
// heartbeatTimeout is the maximum time to wait between heartbeats
// without disconnecting
heartbeatTimeout = 5 * time.Minute
heartbeatJitter = 3 * time.Minute
heartbeatTimeout = 1 * time.Minute
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was the jitter removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

@richardpen Can you explain the removal of the jitter here for posterity?

Copy link
Author

Choose a reason for hiding this comment

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

I have added it back in the new commit after understanding why it was there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reading is hard. Thanks!

@richardpen richardpen force-pushed the reduce-acs-disconnection-period branch from 20f2c51 to 7d33b69 Compare August 3, 2017 20:46
Copy link
Contributor

@nmeyerhans nmeyerhans left a comment

Choose a reason for hiding this comment

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

🚢

    This will reduce the acs disconnection period, the maximum
    disconnection period of acs would be 2 minutes before agent
    try to connect to acs again.
@richardpen richardpen force-pushed the reduce-acs-disconnection-period branch from 7d33b69 to af6e51e Compare August 7, 2017 21:14
@richardpen richardpen merged commit af6e51e into aws:dev Aug 7, 2017
@jhaynes jhaynes mentioned this pull request Aug 22, 2017
@samuelkarp samuelkarp added this to the 1.14.4 milestone Aug 22, 2017
@richardpen richardpen deleted the reduce-acs-disconnection-period branch November 21, 2017 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants