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

fix: use heuristics to determine when to query ddb table ttl #307

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

bruuuuuuuce
Copy link
Contributor

@bruuuuuuuce bruuuuuuuce commented Apr 11, 2024

The describe_time_to_live api call is heavily throttled, we don't want to have to call it for every dynamodb table. This pr separates out the describe ttl call into a separate function, and only calls it on tables who have at least an average of 100tps over the course of a month. We can tune this heuristic later on

Before
Screen Shot 2024-04-11 at 11 49 41 AM

After
Screen Shot 2024-04-11 at 11 36 38 AM

Copy link
Contributor

@cprice404 cprice404 left a comment

Choose a reason for hiding this comment

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

couple of minor questions/nits but overall lgtm

momento/src/commands/cloud_linter/dynamodb.rs Outdated Show resolved Hide resolved
momento/src/commands/cloud_linter/dynamodb.rs Outdated Show resolved Hide resolved
let mut resources = get_ddb_resources(&config, Arc::clone(&limiter)).await?;
let metrics_quota =
Quota::per_second(core::num::NonZeroU32::new(20).expect("should create non-zero quota"));
let metrics_limiter = Arc::new(RateLimiter::direct(metrics_quota));
Copy link
Contributor

Choose a reason for hiding this comment

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

i like this stuff

@bruuuuuuuce bruuuuuuuce merged commit da91e10 into main Apr 15, 2024
3 checks passed
@bruuuuuuuce bruuuuuuuce deleted the fix/speedUpLinter2 branch April 15, 2024 17:02
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.

3 participants