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

Downsize ES domains to save money #2548

Merged
merged 1 commit into from
Dec 5, 2017
Merged

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Dec 5, 2017

There's no need to use such expensive instance types in our tests, so I downgraded these to save some money. 💰

t2.micro.elasticsearch is $0.018 per Hour (the cheapest available w/out EBS)
m3.medium.elasticsearch is $0.094 per Hour (the cheapest available w/ EBS)

r3.large.elasticsearch was $0.245 per Hour

https://aws.amazon.com/elasticsearch-service/pricing/

ES is an expensive leak otherwise and was the most expensive part of our bill - so I'm just picking the low hanging fruit - there's a few other places where we can cut the unnecessary costs.

Test results

=== RUN   TestAccAWSElasticSearchDomain_duplicate
--- PASS: TestAccAWSElasticSearchDomain_duplicate (595.89s)
=== RUN   TestAccAWSElasticSearchDomain_v23
--- PASS: TestAccAWSElasticSearchDomain_v23 (739.38s)
=== RUN   TestAccAWSElasticSearchDomain_policy
--- PASS: TestAccAWSElasticSearchDomain_policy (945.22s)
=== RUN   TestAccAWSElasticSearchDomain_tags
--- PASS: TestAccAWSElasticSearchDomain_tags (997.97s)
=== RUN   TestAccAWSElasticSearchDomain_importBasic
--- PASS: TestAccAWSElasticSearchDomain_importBasic (1029.03s)
=== RUN   TestAccAWSElasticSearchDomain_complex
--- PASS: TestAccAWSElasticSearchDomain_complex (1037.62s)
=== RUN   TestAccAWSElasticSearchDomain_vpc
--- PASS: TestAccAWSElasticSearchDomain_vpc (1373.16s)
=== RUN   TestAccAWSElasticSearchDomain_basic
--- PASS: TestAccAWSElasticSearchDomain_basic (1531.54s)
=== RUN   TestAccAWSElasticSearchDomain_update
--- PASS: TestAccAWSElasticSearchDomain_update (2129.23s)
=== RUN   TestAccAWSElasticSearchDomain_internetToVpcEndpoint
--- PASS: TestAccAWSElasticSearchDomain_internetToVpcEndpoint (2160.52s)
=== RUN   TestAccAWSElasticSearchDomainPolicy_basic
--- PASS: TestAccAWSElasticSearchDomainPolicy_basic (2765.69s)
=== RUN   TestAccAWSElasticSearchDomain_vpc_update
--- PASS: TestAccAWSElasticSearchDomain_vpc_update (2766.88s)

@radeksimko radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Dec 5, 2017
@radeksimko radeksimko force-pushed the t-downgrade-es-domains branch from 8eb7abc to 7483013 Compare December 5, 2017 17:29
@radeksimko radeksimko changed the title Downgrade ES domains to save money Downsize ES domains to save money Dec 5, 2017
Copy link
Contributor

@Ninir Ninir left a comment

Choose a reason for hiding this comment

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

💸 💸 💸 💸 ! 👍

@radeksimko radeksimko merged commit 45c95ba into master Dec 5, 2017
@radeksimko radeksimko deleted the t-downgrade-es-domains branch December 5, 2017 22:38
@tomelliff
Copy link
Contributor

tomelliff commented Dec 11, 2017

@radeksimko not sure if it matters much but a lot of the test resources don't explicitly state the instance type and so get defaulted to m4.larges. For the majority of things these could probably be set to t2.micros I think.

I would expect there to only need a single test case that doesn't set the instance type so you can test that path works nicely and maybe a test case for a different instance type than a t2.micro as well just to test that path is working properly.

I did notice (currently working on the resource in #2632) that the tests just check that the ES domain has been created rather than checking attributes of the resource like I'd expect. Is there a reason behind that or has this just been left off by previous contributors?

@radeksimko
Copy link
Member Author

not sure if it matters much but a lot of the test resources don't explicitly state the instance type and so get defaulted to m4.larges. For the majority of things these could probably be set to t2.micros I think.

Agreed, I was just collecting the low hanging fruit, I didn't notice/realize that.

I did notice (currently working on the resource in #2632) that the tests just check that the ES domain has been created rather than checking attributes of the resource like I'd expect. Is there a reason behind that or has this just been left off by previous contributors?

There is no particular reason I'm aware of. It was most likely just forgotten.

@ghost
Copy link

ghost commented Apr 10, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants