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

Enhanced monitoring: set cloudwatch query maxInterval=60 sec #62

Conversation

vlinevych
Copy link

@vlinevych vlinevych commented Jul 19, 2021

The motivation behind this PR is to reduce the number of requests to Cloudwatch, which are not cheap.
According to the current logic, refresh interval is set to the lowest value of instance.EnhancedMonitoringInterval found among the instances.

Although RDS supports the granularity of 1, 5, 10, 15, 30, or 60, the maxInterval is currently limited to 10 sec only.

In my case, most of my instances are configured for 60sec and 5 out of 6 requests to Cloudwatch return the same data.

I would like to reevaluate the decision that was made back in 2018

Please let me know what do you think.
Thanks.

@vlinevych vlinevych changed the title Bring back maxInterval=60 sec Enhanced monitoring: set cloudwatch query maxInterval=60 sec Jul 19, 2021
Copy link

@JiriCtvrtka JiriCtvrtka left a comment

Choose a reason for hiding this comment

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

For me it is fine. We have 60 seconds intervals also in another places.

@askomorokhov Do you agree?

@JiriCtvrtka JiriCtvrtka requested a review from askomorokhov July 20, 2021 10:06
@vlinevych
Copy link
Author

Thanks for the review!
CI is failing because of missing AWS credentials, there is not much I can do about it.

Error: AWS_ACCESS_KEY and AWS_SECRET_KEY environment variables must be set for this test

I kindly ask some of the collaborators to force-merge it.

@askomorokhov askomorokhov temporarily deployed to CI July 20, 2021 17:41 Inactive
@askomorokhov askomorokhov temporarily deployed to CI July 20, 2021 17:41 Inactive
@askomorokhov askomorokhov temporarily deployed to CI July 20, 2021 17:41 Inactive
@askomorokhov askomorokhov temporarily deployed to CI July 20, 2021 17:41 Inactive
@JiriCtvrtka
Copy link

@denisok Can you merge it? Because of waiting for lint.

@askomorokhov
Copy link

@vlinevych, thank you for your contribution! Merged #63

@vlinevych vlinevych deleted the patch/cloudwatch-increase-maxInterval branch July 22, 2021 16:09
@denisok denisok added community award swag prize for the contributors. Please contact Percona if we can't reach you! labels Dec 1, 2021
@Aleksa-del
Copy link

@vlinevych Hello! We would love to send you a gift from Percona for your contributions. Please, contact us at community-team@percona.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
award swag prize for the contributors. Please contact Percona if we can't reach you! community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants