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

Optimized API call, query timing for the table aws_ec2_instance_type and added support to accept the wildcard pattern(t2*) for instance type. Closes #2292 #2301

Merged
merged 3 commits into from
Dec 17, 2024

Conversation

ParthaI
Copy link
Contributor

@ParthaI ParthaI commented Sep 6, 2024

Integration test logs

Logs
N/A

Example query results

Results
> select instance_type, instance_type_pattern from aws_ec2_instance_type where instance_type_pattern = 'c7i.*'
+----------------+-----------------------+
| instance_type  | instance_type_pattern |
+----------------+-----------------------+
| c7i.16xlarge   | c7i.*                 |
| c7i.xlarge     | c7i.*                 |
| c7i.large      | c7i.*                 |
| c7i.metal-48xl | c7i.*                 |
| c7i.4xlarge    | c7i.*                 |
| c7i.2xlarge    | c7i.*                 |

…nd added support to accept the wildcard pattern for instance type. Closes #2292
@ParthaI ParthaI requested a review from misraved September 6, 2024 12:02
@ParthaI ParthaI self-assigned this Sep 6, 2024
@ParthaI ParthaI changed the title Optimize API call, query timing for the table aws_ec2_instance_type and added support to accept the wildcard pattern(t2*) for instance type. Closes #2292 Optimized API call, query timing for the table aws_ec2_instance_type and added support to accept the wildcard pattern(t2*) for instance type. Closes #2292 Sep 6, 2024
@ParthaI
Copy link
Contributor Author

ParthaI commented Sep 6, 2024

Changes Overview:

API Call Optimization:

  • The table now makes 2 API calls per query, compared to earlier where it made API calls for each row.
  • Query execution time has been reduced by ~75%.

Performance Comparison:

Earlier (73.8s):

> select count(*) from aws_ec2_instance_type;
+-------+
| count |
+-------+
| 11403 |
+-------+

Time: 73.8s. Rows returned: 1. Rows fetched: 11,380. Hydrate calls: 11,403.

Now (12.2s):

> select count(*) from aws_ec2_instance_type;
+-------+
| count |
+-------+
| 11403 |
+-------+

Time: 12.2s. Rows returned: 1. Rows fetched: 11,403. Hydrate calls: 0.

Wildcard Search Support:

  • Added a new column: instance_type_pattern to enable wildcard pattern searches.
  • In the query select * from aws_ec2_instance_type where instance_type = 't2*', the API returned empty rows due to PostgreSQL-level filtering.
    • The instance_type column contains exact values like 't2.small', not 't2*', causing a mismatch between the column value and the wildcard pattern.
    • To address this, the instance_type_pattern column was introduced to allow proper filtering using wildcard patterns.

Example Query:

> select instance_type, instance_type_pattern from aws_ec2_instance_type where instance_type_pattern = 'c7i.*';
+----------------+-----------------------+
| instance_type  | instance_type_pattern |
+----------------+-----------------------+
| c7i.16xlarge   | c7i.*                 |
| c7i.xlarge     | c7i.*                 |
| c7i.large      | c7i.*                 |
| c7i.metal-48xl | c7i.*                 |
| c7i.4xlarge    | c7i.*                 |
| c7i.2xlarge    | c7i.*                 |
| c7i.metal-24xl | c7i.*                 |
| c7i.8xlarge    | c7i.*                 |
| c7i.12xlarge   | c7i.*                 |


Time: 1.3s. Rows returned: 176. Rows fetched: 176. Hydrate calls: 0.

Scans:
  1) aws_ec2_instance_type.aws: Time: 1.2s. Fetched: 176. Hydrates: 0. Quals: instance_type_pattern = 'c7i.*'.

The above query fetched and displayed 176 rows in just 1.3 seconds.


@e-gineer / @misraved, your feedback on the table design and suggestions regarding the column naming for instance_type_pattern would be greatly appreciated.

Thank you!

Copy link

github-actions bot commented Nov 5, 2024

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale No recent activity has been detected on this issue/PR and it will be closed label Nov 5, 2024
@ParthaI ParthaI removed the stale No recent activity has been detected on this issue/PR and it will be closed label Nov 18, 2024
Copy link
Contributor

@misraved misraved left a comment

Choose a reason for hiding this comment

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

@ParthaI what happens if we pass limit 15 to the query? In an ideal case scenario, I wouldn't want to make a batch API call, that passes in 100 instance types.

Please take a look at the minor review comments, thanks!!

}
// Helper function to filter instance types by a pattern like t2-*, m5-*, etc.
func filterInstanceTypesByPattern(ctx context.Context, instanceTypes []types.InstanceType, pattern string) []types.InstanceType {
plugin.Logger(ctx).Error("Pattern =>>>", pattern)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
plugin.Logger(ctx).Error("Pattern =>>>", pattern)

for _, instanceType := range instanceTypes {

if re.MatchString(string(instanceType)) {
plugin.Logger(ctx).Error("Pattern matched with =>>>", instanceType)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
plugin.Logger(ctx).Error("Pattern matched with =>>>", instanceType)

@ParthaI
Copy link
Contributor Author

ParthaI commented Nov 18, 2024

@misraved, thank you for the suggestion! I’ve updated the logic to optimize the API call using the limit query parameter.

@misraved misraved merged commit 7709c95 into main Dec 17, 2024
1 check passed
@misraved misraved deleted the optimize-ec2-instance-type-table branch December 17, 2024 07:57
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