wagtail-cache icon indicating copy to clipboard operation
wagtail-cache copied to clipboard

Keyring in database

Open aweakley opened this issue 1 year ago • 4 comments

This moves the keyring into the database to avoid duplication and also keep it up to date.
It would replace #63

aweakley avatar Feb 12 '24 05:02 aweakley

Very nice implementation. Thank you! I left a few comments. Also, another bug fix has been merged in recently (adds a try/catch in UpdateCacheMiddleware so you may need to rebase to resolve those conflicts).

vsalvino avatar Jul 16 '24 16:07 vsalvino

I've just reverted this to draft because we're finding that calling clear_expired() during set() is not thread safe.

aweakley avatar Aug 22 '24 08:08 aweakley

Interesting - are you using ASGI by chance? Or is this caused by concurrent calls to clear_expired()?

vsalvino avatar Aug 22 '24 14:08 vsalvino

It's not ASGI, but there are lots of servers and workers. We were getting many concurrent requests creating calls to set and they were each creating a clear_expired query on the database. Because that was backing up the pages were not being put into the cache quickly, so that led to even more requests creating calls to both things. We were finding a lot of pages were expiring at the same time, I guess on the anniversary of when the cache was last cleared. So I added a way to add some random variation to the timeout which smoothed things out a lot but it didn't fix it. In the end the fix turned out to be to run ANALYZE; on the database and everything became fast again (for some reason auto vacuum didn't catch it) - so a completely separate issue than all the things I'd thought of up to then. So now with the database back to normal I think clear_expired on set would probably be fine, but I've still kept the option to have it run separately.

I'll tidy it up and re-submit the PR.

aweakley avatar Oct 23 '24 07:10 aweakley