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

refactor & improvements #42

Merged
merged 8 commits into from
Jan 27, 2020

Conversation

eytan-avisror
Copy link
Collaborator

@eytan-avisror eytan-avisror commented Jan 21, 2020

Contributes to #35

This PR refactors a lot of the code for better structure and better performance of API Calls to AWS.

  • Logging improvements
  • Deregistering improvements - calls are now made across instances instead of per target per group. (great performance improvement)
  • Use inverse-exponential-backoff for waiters
  • Improved error handling for goroutines
  • caching improvements (don't flush on Deregister)

Testing Done:

  • tested performance on 35 nodes concurrently killed on 250 tg, 0 errors, avg. 1500 seconds
  • tested performance on 50 nodes concurrently killed on 250 tg, 0 errors, avg. 2800 seconds
    see test results in last comment

@eytan-avisror eytan-avisror requested a review from a team as a code owner January 21, 2020 01:42
@codecov
Copy link

codecov bot commented Jan 21, 2020

Codecov Report

Merging #42 into master will increase coverage by 0.24%.
The diff coverage is 69.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #42      +/-   ##
==========================================
+ Coverage   72.82%   73.06%   +0.24%     
==========================================
  Files           9       12       +3     
  Lines         861      995     +134     
==========================================
+ Hits          627      727     +100     
- Misses        176      206      +30     
- Partials       58       62       +4
Impacted Files Coverage Δ
pkg/service/lifecycle.go 61.53% <ø> (ø)
pkg/service/nodes.go 86.58% <ø> (-0.17%) ⬇️
pkg/service/elbv2.go 100% <100%> (ø) ⬆️
pkg/service/events.go 78.57% <100%> (-2.68%) ⬇️
pkg/service/elb.go 100% <100%> (ø) ⬆️
pkg/service/metrics.go 84.61% <33.33%> (-15.39%) ⬇️
pkg/service/deregistrator.go 47.88% <47.88%> (ø)
pkg/service/autoscaling.go 78.57% <60%> (ø) ⬆️
pkg/service/server.go 62.82% <66.44%> (+0.46%) ⬆️
pkg/service/target.go 78.26% <78.26%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44e79ed...bf6f9a2. Read the comment docs.

@eytan-avisror
Copy link
Collaborator Author

Load tests:
Screen Shot 2020-01-27 at 11 04 07 AM
Screen Shot 2020-01-27 at 11 10 37 AM

Copy link
Collaborator

@shrinandj shrinandj left a comment

Choose a reason for hiding this comment

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

Can you add a "Testing Done" section to the commit? For large PRs, that's the easiest way to check if all corner cases have been covered.

@eytan-avisror
Copy link
Collaborator Author

Can you add a "Testing Done" section to the commit? For large PRs, that's the easiest way to check if all corner cases have been covered.

There are test results one comment above, I've added a summary in the PR

@eytan-avisror eytan-avisror merged commit 8ff39ca into keikoproj:master Jan 27, 2020
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.

2 participants