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

Can't flush tags on Redis cluster cache in Laravel 5.1.30 and later #17713

Closed
mattmant opened this issue Feb 1, 2017 · 34 comments
Closed

Can't flush tags on Redis cluster cache in Laravel 5.1.30 and later #17713

mattmant opened this issue Feb 1, 2017 · 34 comments

Comments

@mattmant
Copy link

mattmant commented Feb 1, 2017

  • Laravel Version: 5.1.45
  • PHP Version: 5.5.9
  • Redis Version: 3.2.6

Description:

I updated Laravel from version 5.1.20 to version 5.1.45. I'm using cache Redis with tags to store some informations. The application is running on 3 web-servers and Redis is in cluster mode.

After the update I can't flush tags saved in Redis because I get the following error:

NotSupportedException in RedisCluster.php line 380:
Cannot use 'DEL' with redis-cluster.

The important thing is that before updating Laravel the application was stable and was working well.

I saw that there was many important changes into Illuminate\Cache\RedisTaggedCache with the introduction of $this->deleteStandardKeys(); into the flush() method.

Steps To Reproduce:

You can test the behavior in your local machine with a single Redis node (the same happens if you have a real Redis cluster also).

  1. Set Redis in cluster mode by putting 'cluster' => true in your Redis config in config/database.php . For example:
'redis' => [
    'cluster' => true,
    'default' => [
        'host' => '127.0.0.1',
        'port' => 6379,
        'database' => 0
    ]
]
  1. Store at least two keys for the same tag:
Cache::store( "redis" )->tags( "tag" )->put( "key1", "test", 100 );
Cache::store( "redis" )->tags( "tag" )->put( "key2", "test", 100 );
  1. Now try to flush the tag:
    Cache::store( "redis" )->tags( "tag" )->flush();

You will get the following red message:
Predis\NotSupportedException with message 'Cannot use 'DEL' with redis-cluster.'

@mattmant
Copy link
Author

mattmant commented Feb 2, 2017

Update: the issue was introduced with the 5.1.30 release of laravel/framework.

@taylorotwell
Copy link
Member

You are seeing a Predis exception. I'm not sure there is much we can do about it? Perhaps asks on their repository?

@eldair
Copy link

eldair commented Feb 5, 2017

Same thing with 'cluster' => false in 5.4

@mattmant
Copy link
Author

mattmant commented Feb 6, 2017

@taylorotwell yes, it is a Predis exception and it would be a good idea to ask in their repository, after have reproduced the behaviour directly on it.

But, updating Laravel from 5.1.20 to 5.1.45 didn't affect Predis package version: it still remains v1.1.1.
It seems that after changes on Illuminate\Cache\RedisTaggedCache it doesn't properly support redis cluster... because my application was stable and was working well before these changes.

@mattmant
Copy link
Author

This issue should be tagged for any version of Laravel because of wrong definition of tags into Illuminate\Cache\RedisTaggedCache.

Refer to this #17792 (comment) for more informations, and this about what could be changed: https://github.com/laravel/framework/pull/17792/files.

@tillkruss
Copy link
Collaborator

Your updated #17792 PR that uses hash tags looks good to me. If it doesn't get merged into 5.1, I'd at least submit it for 5.4.

@eldair
Copy link

eldair commented Feb 13, 2017

Is this moving forward?

@mattmant mattmant changed the title [5.1] Can't flush tags on Redis cluster cache after updating to Laravel 5.1.45 Can't flush tags on Redis cluster cache in Laravel 5.1.30 and later Feb 16, 2017
@ClaraLeigh
Copy link

Still no fix or workaround for this bug?

@benjamincrozat
Copy link

So it seems like this bug is the same as the one I posted (#22788). Does anybody could take a look? @taylorotwell @themsaid

@tillkruss
Copy link
Collaborator

I'd suggest to submit a PR for v5.6.

@benjamincrozat
Copy link

Will do that once I have some time. I'm not really familiar with the code base.

@ghost
Copy link

ghost commented Feb 7, 2018

Had the issue too. Have the same app on a redis 3.2.8 don't reproduce the error.

@T0miii
Copy link

T0miii commented Apr 3, 2018

Any update on this?

Getting similar error wthile trying to flush a tag. Also using Redis Clusters.

@madsem
Copy link

madsem commented Apr 18, 2018

@mattmant I wonder how this still isn't merged :(
Is there any alternative right now to be able to do Cache::tags('something')->flush() with Redis Cluster enabled?

I'm on 5.6 btw and have the same issue.

@GrahamCampbell
Copy link
Member

I wonder how this still isn't merged :(

This is not a pull request. There's nothing to merge?

@madsem
Copy link

madsem commented Apr 18, 2018

Okay sorry for not using 100% accurate verbiage @GrahamCampbell , but in the end this still doesn't work in 5.6 and cannot flush tagged cache with redis cluster.

@GrahamCampbell
Copy link
Member

We're open to pull requests to fix this.

@madsem
Copy link

madsem commented Apr 18, 2018

That's what I meant, there was one a year ago for 5.1 https://github.com/laravel/framework/pull/17792/files

;)

@devcircus
Copy link
Contributor

Taylor mentioned on that pull request, that something similar may be considered for master. Maybe someone just needs to PR it?

@tillkruss
Copy link
Collaborator

Yeah, just send a PR to v5.7.

@danygiguere
Copy link

I haven't found a solution but maybe a way around it for some cases. In my case I was tagging single posts with this tag: Cache::tags(["post_$postId"])and I had a problem when I was trying to clear the cache for a user’s posts like so

foreach($user->posts() as $post) {
Cache::tags(["post_$post->id"])->flush();
}

Somehow the bug didn't occurred all the time. Within some methods the foreach worked properly. Only in one method it failed.

I found a solution by adding this tag "post_user_$post->user->id"
So now I have two tags:

Cache::tags(["post_user_$post->user->id", "post_excerpt_$postId"])

and I can clear the cache for those posts with:
Cache::tags(["post_user_$user->id"])->flush();

@cameronpalatas
Copy link

Any update on this? Running into the same issue.

@driesvints driesvints changed the title Can't flush tags on Redis cluster cache in Laravel 5.1.30 and later [5.1] Can't flush tags on Redis cluster cache in Laravel 5.1.30 and later Oct 2, 2018
@driesvints driesvints added LTS and removed LTS labels Oct 2, 2018
@laurencei
Copy link
Contributor

Can anyone confirm this is occuring on Laravel 5.7 (or 5.5)? Would be good to know if any of the recent updates fixed this?

@zeetabit
Copy link

zeetabit commented Oct 9, 2018

same error :-(

@cg-rsands
Copy link

Same problem, anyone any idea?? @taylorotwell

@driesvints driesvints added the bug label Nov 23, 2018
@vismutx
Copy link

vismutx commented Nov 26, 2018

So, how to use tags with this bug? any way around it?

@anthonyvancauwenberghe
Copy link

So, how to use tags with this bug? any way around it?

use phpredis instead of predis if you want to go around it

@IbrahimFathy19
Copy link

Any updates on this issue?
if not, then I'm ready to work on it.

@driesvints driesvints removed the lts label Nov 14, 2019
@Jafo232
Copy link

Jafo232 commented Jan 21, 2020

This is still an issue with Memcached..

@driesvints
Copy link
Member

@Jafo232 we're welcoming prs to help solve this.

@driesvints driesvints changed the title [5.1] Can't flush tags on Redis cluster cache in Laravel 5.1.30 and later Can't flush tags on Redis cluster cache in Laravel 5.1.30 and later Mar 17, 2020
@taylorotwell
Copy link
Member

It's still an issue because nobody has fixed it. Simple as that. 😄 Anyone can fix it by making a PR to the framework.

@taylorotwell taylorotwell removed their assignment Apr 23, 2020
@taylorotwell
Copy link
Member

I have confirmed this is working on a Redis cluster I provisioned on ScaleGrid.io and using the phpredis PHP extension (default Redis driver for Laravel now). If it is not working on Predis I'm not sure what to do about that. Predis is no longer maintained as a library.

@Nuranto
Copy link

Nuranto commented Apr 30, 2020

I have the issue with memcached too.

edit : so I switched to redis (using php-redis) and it works fine... Maybe memcached should not be authorized to use tags, as it is the case for file or database.

@mattmant
Copy link
Author

mattmant commented May 1, 2020

I have confirmed this is working on a Redis cluster I provisioned on ScaleGrid.io and using the phpredis PHP extension (default Redis driver for Laravel now). If it is not working on Predis I'm not sure what to do about that. Predis is no longer maintained as a library.

@taylorotwell As I have suggested three years ago, I'm still using the following solution, without any side effects on performance/data: https://github.com/laravel/framework/pull/17792/files

It is not a BC as you said here #17792 (comment). It is a simply way to tell to predis how to manage associative keys. Redis keys and the way they are defined/stored will not be changed.

For more information:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests