-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
EC2 autoscaler: avoid hitting aws filter limits #1954
Conversation
@@ -45,6 +46,7 @@ | |||
public class EC2AutoScaler implements AutoScaler<EC2EnvironmentConfig> | |||
{ | |||
private static final EmittingLogger log = new EmittingLogger(EC2AutoScaler.class); | |||
public static final int MAX_AWS_FILTER_VALUES = 100; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this documented by AWS somewhere? if so can we put a comment where this came from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't searched the docs, but the limit appears to be 200 by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we also add a comment ?
👍 |
1 similar comment
👍 |
hold on before merging, there's another place where we might hit this limit |
0a80ba2
to
d42c8b3
Compare
Still 👍 |
ok I added comment, and fix the other filter query as well, please have a look |
I'm guessing there's no way to check this in a unit test is there? |
@drcrallen I can add a unit test, sure, |
d42c8b3
to
065dfd0
Compare
065dfd0
to
749ac12
Compare
added tests |
👍 |
EC2 autoscaler: avoid hitting aws filter limits
Prevents the following error below from happening.
As a side benefit this also materialized the instanceId list, so it should reduce garbage creation.