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

Cache submission overview and list pages #1158

Closed
wants to merge 1 commit into from

Conversation

Parbhat
Copy link
Contributor

@Parbhat Parbhat commented Apr 11, 2019

Link #949

Initial cache for submission overview (/apply/submissions/) and list pages (/apply/submissions/all/). The cache will be specific to the user as add Vary by Cookie header. And pages will be cached for 30 mins. This is the initial trial and can extend further timing and more pages.

The first request to the page will be normal and subsequent requests will be served from cache. For testing locally, the backend should be changed from Dummy cache in local.py. Dummy cache doesn’t actually cache – it just implements the cache interface without doing anything.

@Parbhat Parbhat requested a review from frjo April 11, 2019 10:09
@frjo
Copy link
Member

frjo commented Apr 11, 2019

@Parbhat How is cache invalidation handled? As soon as a submission or any related content to it is updated the cache needs to be updated.

With proper invalidation we could set a longer cache time as well.

@Parbhat
Copy link
Contributor Author

Parbhat commented Apr 11, 2019

@frjo The current cache is specific to a user. So, a user 1 will have a separate cache from user 2. We can burst a user cache when submission is updated for a submission that can be accessed by the user. Also, the cache is dependent on user cookie. Signing out and login will also burst the cache. Caching based on user helps to cache table data permitted to user and show correct username in menu.

@frjo
Copy link
Member

frjo commented Apr 11, 2019

Yes, doing it by user is necessary I believe with the permissions we have in place.

But when staff is working with the system they will expect everything to be up to date. Status, number of reviews etc. needs to be updated directly if changed.

Can we tag caches with content id:s? That way we could clear all caches by id when thet content is updated.

@frjo
Copy link
Member

frjo commented Apr 11, 2019

Maybe we can start by implementing cache for the public pages and the apply front page? They are easier to handle I believe.

@Parbhat
Copy link
Contributor Author

Parbhat commented Apr 11, 2019

@frjo Currently we are using per-view cache. It caches the complete page. We can also use low-level cache API to control the bits that should be cached. And on some signal like submission update burst the cache by key.

For public pages, this package looks good - wagtail-cache. I will check if it fits our use case.

@Parbhat
Copy link
Contributor Author

Parbhat commented Apr 11, 2019

@frjo Should we cache public pages server side or just send the cache-control response header with max-age, s-maxage?

@frjo
Copy link
Member

frjo commented Apr 11, 2019

Also cache them, not everyone running this system will have a CDN in front.

Sent with GitHawk

@Parbhat Parbhat closed this Apr 18, 2019
@frjo frjo deleted the 949/cache-overview-and-list-pages branch February 25, 2020 21:27
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.

2 participants