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

Add shared tracks cache for get_tracks #923

Merged
merged 3 commits into from Oct 13, 2020
Merged

Conversation

piazzatron
Copy link
Contributor

@piazzatron piazzatron commented Oct 12, 2020

Description

Adds shared caching for get_tracks, so we get at leat some level of caching for bulk fetches.

This gets rid of the existing get_unpopulated_tracks caching function.

Services

  • Discovery Provider
  • Creator Node
  • Identity Service
  • Libs
  • Contracts
  • Service Commands
  • Mad Dog

Does it touch a critical flow like Discovery indexing, Creator Node track upload, Creator Node gateway, or Creator Node file system?

  • ✅ Nope

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.
Include log analysis if applicable.

Tested locally against prod, made sure we returned expected results + hit cache when expected.

@piazzatron
Copy link
Contributor Author

piazzatron commented Oct 13, 2020

Any harm you can think of from removing the cache layer that I did here?

discovery-provider/src/queries/get_unpopulated_tracks.py Outdated Show resolved Hide resolved
@@ -91,8 +100,7 @@ def get_unpopulated_track():

return (tracks, track_ids)

key = make_cache_key(args)
(tracks, track_ids) = use_redis_cache(key, UNPOPULATED_TRACK_CACHE_DURATION_SEC, get_unpopulated_track)
(tracks, track_ids) = get_tracks_and_ids()
Copy link
Member

Choose a reason for hiding this comment

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

should we be adding to cache in the event that we do get stuff from the db here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

discussed on slack

@piazzatron piazzatron merged commit 4f823b4 into master Oct 13, 2020
@piazzatron piazzatron deleted the piazz-tracks-api-caching branch October 13, 2020 00:59
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.

None yet

2 participants