Skip to content

Conversation

@pbearne pbearne requested a review from spacedmonkey January 4, 2022 20:58
@sboisvert
Copy link

@pbearne I worry about the flush cache group automatically flushing everything. I like the approach here: pantheon-systems/wp-redis@db32e94 that only flushes if it's possible to target them. It feels like a safer approach.

What we could do is, separately from the changes to core. Add support for groups to object cache libraries. The redis one above already implements it but I don't think any memcache ones do. I suspect it's because memcache has no concept of groups and therefore it's just a hash of the concatenation of the key and group most of the time, but we could pseudo implement it by having groups be a pointer to an incrementor and when you flush the cache group you're in essence just incrementing the group part of the cache key, which while not actually flushing the keys, would make them return false / repopulate them.

added filter to allow plugin to adjust the called function
@pbearne
Copy link
Author

pbearne commented Jan 7, 2022

@sboisvert I have removed the failback
I have refactored the functions to make them more SOLID
I will need to add some more tests for the new functions but I through I get the code/approach sorted first
Should a change the method to detect and function names to redist example?

@spacedmonkey
Copy link

You might want to revert these commits and just merge trunk into this branch. I am seeing a lot of changes that are unrelated to this PR.

pbearne added 2 commits March 24, 2022 11:28
removed WP_Object_Cache_dummy class
removed WP_Object_Cache_dummy class
Copy link

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

This patch looks solid now. Let's submit it to WP Core team and get feedback from the wider community.

pbearne and others added 19 commits April 5, 2022 12:33
And flush all if flush_group not supported
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.

4 participants