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

Allow options to consider c5 and c4 instances equal #206

Open
binarylogic opened this issue Jan 15, 2018 · 7 comments
Open

Allow options to consider c5 and c4 instances equal #206

binarylogic opened this issue Jan 15, 2018 · 7 comments

Comments

@binarylogic
Copy link
Contributor

Github issue

Issue type

  • Feature Idea

Summary

When a c5.* instance is specified as the on-demand instance, c4.* instances are never used because they contain 500mb less than their c5 counterpart. The autospotting algorithm excludes c4 instances it due to this memory difference.

Expected results

This is technically correct, and I'm on the fence if this should be the default behavior, but most would expect a c4 instance to be a valid replacement for a c5 instance.

@cristim
Copy link
Member

cristim commented Jan 15, 2018

Good catch, we can relax the memory comparison algorithms to accept a small difference if people expect this.

The problem is that those 500M are pretty much negligible for larger instance types but this is not okay for the smallest micro and nano instances where this makes a lot of difference.

We could make it a default as long as we carefully set it to not cover the smaller instances. Maybe accept a difference in both percentage(up to 15-20%) and absolute value of 500M, whichever is smaller.

@binarylogic
Copy link
Contributor Author

Yep, that was exactly my thought. c5.* instances offer exactly 93.75% of the memory that c4.* instances have available. I'm thinking we allow an even 10% difference.

@cristim
Copy link
Member

cristim commented Jan 16, 2018

Looking forward for a PR addressing this

@russellballestrini
Copy link
Contributor

russellballestrini commented Feb 8, 2018

I can duplicate this issue, but c4 also is not replaced by c4 so I think there is a bigger defect at the root.

2018/02/08 03:09:23 instance.go:273: Comparing c4.2xlarge with c5.2xlarge
2018/02/08 03:09:23 instance.go:116: Comparing price spot/instance:
2018/02/08 03:09:23 instance.go:120: EBS Surcharge : 0
2018/02/08 03:09:23 instance.go:123: Spot price: 0
2018/02/08 03:09:23 instance.go:124: Instance price: 0.34

As you can see, there is no pricing data for c4 (which is the same problem for c5)

@russellballestrini
Copy link
Contributor

russellballestrini commented Feb 9, 2018

This is still an issue but as a work around you can use the allowed_instance_types option to pass a list of valid instance types. I have verified that this works as a work around.

https://github.com/cristim/autospotting/blob/master/START.md#testing-configuration

EDIT (@xlr-8): updated to point to a documentation link rather than commits lines more subject to changes.

@gabegorelick
Copy link
Contributor

Similar issue happens for EBS, which is mentioned in #353. If you have a t3 instance, t2 will not be considered since it lacks EBS throughput. This will probably continue to be an issue in the future as newer instance types add new features that older instances types will naturally lack.

I think educating users about autospotting_allowed_instance_types is a fine solution though.

@cristim
Copy link
Member

cristim commented Mar 6, 2023

For EBS we now have a flag that makes the comparison optional.

The issue with C4 and C5 is the memory size difference. This should be easy to fix, although nowadays few people use C3 and older instances anymore.

@gabegorelick @russellballestrini, @binarylogic: are you guys still interested in this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants