swayok / alternative-laravel-cache

Replacements for Laravel's redis and file cache stores that properly implement tagging idea
MIT License
153 stars 24 forks source link

When cache-key contains a pipe character, forget() function is clearing based on a partial match #31

Closed roberttolton closed 3 years ago

roberttolton commented 3 years ago

Laravel ^7.30.1 Alternative Cache ^5.4

Steps to reproduce (using tinker):

// Put key with colons
Cache::put('cache-key:something:something-else', 'blah', now()->addHours(6));

// Put key with pipes
Cache::put('cache-key|something|something-else', 'blah', now()->addHours(6));

// Get key with colons
Cache::get('cache-key:something:something-else');
"blah"

// Get key with pipes
Cache::get('cache-key|something|something-else');
"blah"

// Forget call
Cache::forget('cache-key');

// Get key with colons
Cache::get('cache-key:something:something-else');
"blah"

// Get key with pipes
Cache::get('cache-key|something|something-else');
null

The call to Cache::get('cache-key|something|something-else'); should not return null as it wasn't the whole correct key used in the forget command.

Have tested this on a Laravel 8 install without Alternative Cache installed, and it's not a problem. Either it's been fixed in Laravel 8 (couldn't find any mention) or this package seems to be clearing based on a partial match with pipes?

swayok commented 3 years ago

@roberttolton pipe character is indeed a special case. Here is the details - http://www.php-cache.com/en/latest/hierarchy/. In short - pipe character is used as hierarchy separator. That's why it clears whole hierarchy with Cache::forget('cache-key');

roberttolton commented 3 years ago

@swayok Ah I see, thanks for the link, that clears that up.

The reason I ultimately stumbled upon this is because I had some cache-warming techniques for Controller routes, that would clear and rebuild caches when a model event happened. The caches were keyed with a main key imploded using a pipe character along with other request variables

When I was forgeting anything with just my main key, this meant the imploded key caches were also cleared (as I know now), and then some other chain of events that meant my cache grew until Redis ran out of memory, and started causing all sorts of other issues. I'm not really sure what made it grow, maybe they weren't fully forgotten or something with the expiry TTL got messed-up?

I can try and make an example that's non-specific to my App if that helps, or if the fix is a simple "don't use pipes" (I've swapped to colons now) then no worries. But I can basically make my App cache act normal or swell-up simply by swapping between colons or pipes.

swayok commented 3 years ago

@roberttolton I've updated readme to warn about pipe specifics. Actually hierarchial cache + redis might work not as good as tagged cache. It was designed for file-based cache in first place and there are no need for any metadata while for redis metadata is required. I have never tested how it works in real life projects and cannot say if it caused redis going out of memory but it definitely might be a reason. I'm using tags currently and they are good enough. Haven't experienced any issues with tags yet. In your case - just use any other separator except | and it will be fine.

roberttolton commented 3 years ago

@swayok Yeah I don't need to use the pipe, I was just looking for any separator to use. A colon is working fine for me now. No problem with using the tags, have been using it for a while in a few projects and works in the correct way (as I see it) that tags should work, not Laravel's default weird tagging strategy - so thanks for the work on the package!

swayok commented 3 years ago

@roberttolton you are welcome! =)