User cache purge #3189
User cache purge #3189
Conversation
warehouse/packaging/__init__.py
Outdated
| purge_keys=["project/{obj.normalized_name}", "all-projects"], | ||
| cache_keys=[ | ||
| key_factory("project/{obj.normalized_name}"), | ||
| key_factory("user/{itr.username}", iterate_on='users'), |
There was a problem hiding this comment.
What changes to a user would get reflected on pages that use the Project model?
There was a problem hiding this comment.
Technically if the user changes their primary email, their avatar would change, which is shown on the "Maintainers" section for project/release pages.
Also if we ever ship #1190, the URLs to maintainers would be wrong.
There was a problem hiding this comment.
My primary concern is that there is a limit to how many surrogate keys (what these are) we can attach to a single HTTP response. Since we have no control over how many users can get added to a single project, that would mean that a page may have more surrogate keys than we can support in a single response (and I believe what happens then is that we just silently lose some of the surrogate keys).
I think probably it doesn't make sense to support iterate_on for cache keys because we can't control how many keys are going to get added. However iterate_on for purge keys makes total sense, and this example here could easily be implemented by, instead of adding a cache key for user to each page, adding a purge key to User that is something like key_factory("project/{itr.normalized_name}", iterate_on="projects"). That should have the same effect, without the unbounded surrogate key problem.
There was a problem hiding this comment.
Oh, and if you make that change, add a comment please about why one supports it and one doesn't, otherwise it'll probably be an attractive nuisance for people wanting to add support for it to cache keys and relying on tribal knowledge for why.
warehouse/packaging/__init__.py
Outdated
| purge_keys=["project/{obj.project.normalized_name}", "all-projects"], | ||
| cache_keys=[ | ||
| key_factory("project/{obj.project.normalized_name}"), | ||
| key_factory("user/{itr.username}", iterate_on='project.users'), |
There was a problem hiding this comment.
Same question as above, what change to a user requires invalidating the cache for anything that uses the Release model?
This also makes it impossible to use the `key_factory` and `iterate_on` with cache keys, to avoid the problem of unbounded surrogate keys.
We don't need to do this per-project to purge the cache anymore.
|
@dstufft This is ready for re-review. |
Fixes #2861.
This PR enables purging pages related to the
Usermodel whenever either theUsermodel changes, or any projects/releases related to that user.To do so, we need to be able to generate cache keys from some iterable attribute on the changed object (like
Project.usersorRelease.project.users).