Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add pluggable module for background update control #11261

Closed
erikjohnston opened this issue Nov 5, 2021 · 5 comments · Fixed by #11306
Closed

Add pluggable module for background update control #11261

erikjohnston opened this issue Nov 5, 2021 · 5 comments · Fixed by #11306
Assignees
Labels
T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@erikjohnston
Copy link
Member

Background updates are controlled by a fairly simple class that ensures that a background update only runs for 100ms every second. This works for the common case, but other deployments (such as those using RDS) may want to use other metrics to control the background updates (e.g. RDS burst metrics).

@erikjohnston erikjohnston added the T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. label Nov 5, 2021
@daenney
Copy link
Contributor

daenney commented Nov 5, 2021

For the AWS RDS Burst Balance metric, as an example:

$ aws --region=<REGION> cloudwatch get-metric-statistics --metric-name BurstBalance --dimensions Name=DBInstanceIdentifier,Value=<RDS instance name> --statistics=Average --namespace=AWS/RDS --period=300 --start-time 2021-11-05T09:00:00Z --end-time 2021-11-05T09:20:00Z

Period is the interval within start-time and end-time. In this example you'll get 4 data points since I'm asking for 300s (5min) at a time over a 20min window.

The AWS API is named the same as the CLI command, but CamelCased

@daenney
Copy link
Contributor

daenney commented Nov 5, 2021

If we had some kind of "pacer" interface for controlling background updates you could ship the current 100ms at a time but plug in a different one through a module.

@richvdh
Copy link
Member

richvdh commented Nov 5, 2021

Authentication will require a token. One option is to replicate what the s3 storage provider does (just an access_key_id and secret_access_key in the config dict), but @daenney suggests it would be nice to replicate the behaviour of the AWS sdks, and read from ~/.aws or AWS_* env vars.

update: boto3 (which we use for the s3 storage provider) supports reading the aws creds from the normal places anyway, so it should all just work.

@daenney
Copy link
Contributor

daenney commented Nov 5, 2021

If you want to provide additional config options in this module I think that's fine, but we should always allow fallback to boto3's built-in credential discovery. It's consistent across the AWS SDKs and is what a lot of folks will expect. It can do a very useful things like assume role providers and using instance metadata service to use IAM roles which avoids needing to ship credentials in the first place.

@daenney
Copy link
Contributor

daenney commented Nov 5, 2021

In the case of RDS it might be worthwhile to grab the "Minimum" instead of "Average" statistic, so it throttles a bit more aggressively (or make that configurable, but you want to be careful to not end up in a scenario of averaging averages).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants