[5.1] Fix for deleting tag keys in Redis cluster

[mattmant]

This is a fix proposal for the issue documented in #17713.

I would ask for @mstephens review to get his expertise and about his last implementations on Illuminate\Cache\RedisTaggedCache.

[taylorotwell]

This sounds like a performance nightmare?

[vlakoff]

You may want to backport #16568. It fixes the same issue, in a better way, using chunks.

[mattmant]

@taylorotwell sure it seems to be

@vlakoff thanks but it isn't the solution to the issue explained in #17713

I contacted @nrk, Predis author, who gave me some important informations and linked me to:
http://stackoverflow.com/questions/38042629/redis-cross-slot-error
and especially:
https://redis.io/topics/cluster-spec#keys-hash-tags

I just made some changes on Illuminate\Cache\RedisTaggedCache by adding the support to "hash tags" and removing the previous nightmare addition:
mattmant@cae98b6.

I made some tests on my environments and it seems to work fine.
Obviously it is needed to revise the test suite, but I would like to know some opinions before it.

[vlakoff]

you forgot to undo this

[mattmant]

Thanks!

[taylorotwell]

Yes, that change could work but honestly I wouldn't want to make it on 5.1. It would be a breaking change. It would be something to be considered for 5.5 perhaps.

[mattmant]

@taylorotwell It would break stored tagged keys because changing their prefix they won't be recognized. All this in a non-persistent caching system.

But would be an important fix for changes occurred with 5.1.30 which bring good implementations but completely compromise the use of tagged keys on Redis cluster (strangely it works before).

Right now I can't upgrade an LTS above 5.1.29 because of broken changes...

[FabryB]

@taylorotwell someone already did a breaking change on 5.1.30, this is merely a fix.
5.1 is still LTS, right ?

[nrk]

When Predis is connected to a cluster, you could just group keys by the slot they are associated to by using something along these lines. You will still end up having multiple DEL commands issued, but at least a single slot holding more than one key will require only one DEL command. This doesn't change anything in the actual key-name scheme, so it's not a BC of sort (the use of hash-tags in my snippet is just to demonstrate what happens with slots holding more than one key).

[FabryB]

Actual Laravel implementation of tags is not using the curly braces, so instead of {tag1}:abc key name is tag1:abc and this is causing the problem

[mattmant]

@FabryB yes this is the main issue

@nrk solution's heads toward group keys and launch DEL command for each group. In this way will not be thrown Predis\NotSupportedException with message 'Cannot use 'DEL' with redis-cluster.'. It won't be a breaking change but maybe not simple to implements.
Anyway would be a workaround, instead to define tags in the right way with { and } characters.

[nrk]

@FabryB as I've clarified in my comment my solution doesn't require the use of curly braces, the hash-tags (in redis lingo it's the portion of a key between curly braces that is used to compute its hash) in that snippet are just there to easily reproduce the case of more than one key associated to the same slot.