-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: add caching #150
feat: add caching #150
Conversation
114916d
to
eca3a5d
Compare
src/grpc/charts.rs
Outdated
if chart.was_cached { | ||
info!( | ||
"Using cached chart data for category '{:?}' in timeframe '{:?}'", | ||
category, timeframe | ||
); | ||
} |
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.
We said we'd look into the top level task periodically logging cache metrics at a later date but are we wanting to keep this info level logging each time a cached chart is served?
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.
Ah sorry, this made it's way back in during the rebase - will remove it for now
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'm assuming that the new trait bounds on Category, Timeframe and Chart are required by cached
? Other than the comment around the logging this all looks good to go 👍
Yes, adding |
Adds some simple caching logic to prevent too many unnecessary db requests, constant re-calculation of the top charts and spamming snapcraft.io with requests to resolve snap IDs.
get_chart
, cached for 24 hours)snap_name
s (get_snap_name
, cached indefinitely, currently used in the category updates, will also be used for the charts when migrating from IDs to names)get_by_snap_id
, cached for 24 hours)Personal vote data (
get_all_by_client_hash
) is not cached.Caching can be disabled at compile time (for integration tests) via the
SKIP_CACHE
environment variable.UDENG-5543