colinmollenhour / Cm_Cache_Backend_Redis

A Zend_Cache backend for Redis with full support for tags (works great with Magento)
Other
390 stars 142 forks source link

Warning: md5() by cluster #136

Closed gabi77 closed 5 years ago

gabi77 commented 5 years ago

In the server logs I have the following errors: Cluster Elasticache by AWS.

2018-11-27T08:48:45+00:00 ERR (3): Warning: md5() expects parameter 1 to be string, array given in .../lib/Credis/Cluster.php on line 260

Have you ever encountered this problem.

Possibly I have 1 primary node and 2 secondary node

alberts-s commented 5 years ago

We are also seeing this when using Elasticache configuration with at least two slave nodes. It appears that in the case there is only one slave node the byHash function is not triggered at all and therefore the error does not occur.

colinmollenhour commented 5 years ago

Do you know what command is being called when this warning is thrown? There is a list of commands that skip the hashing and it could be that the command needs to be added to the list. Or it could be that the array should be iterated can called separately for each client.

alberts-s commented 5 years ago

I cannot confirm myself, however one of our developers indicated that the command which is possibly causing issues is SINTER.

colinmollenhour commented 5 years ago

This Credis_Cluster class is not meant to handle that type of query, it is just a simple wrapper that directs single-key operations to the right node. As that is a multi-key operation it is not supported and you'd need to do the intersection application-side.

There is now the official Redis Cluster which might be more suitable although I don't know if it supports SINTER either as I've not read more than the intro.

alberts-s commented 5 years ago

The problem is that the issue is triggered even if cluster mode is not being used, using the Elasticache (master-slave) configuration shown here.

I might be misunderstanding something, therefore please pardon my ignorance, but why are the keys being hashed at all if cluster mode is not being used?

colinmollenhour commented 5 years ago

Looking back at that code I don't think it is a very good implementation.. Credis_Cluster is designed for sharding, but the <cluster> option looks like it is for master-slave and read distribution. If you have only one slave or round-robin DNS available to you then I suggest using the <load_from_slave> option instead. Or, if you have Sentinel you can use the <load_from_slaves> option so that the list of slaves is read from Sentinel and one is chosen randomly to maintain even distribution.

alberts-s commented 5 years ago

Thanks for the explanation and looking into it! Unfortunately in the case of AWS Elasticsearch Redis there is no managed round robin DNS available, however that does not mean it is not possible to implement it ourselves in AWS. :)

What are your thoughts on removing the Elasticache documentation and feature set altogether? Since as it currently stands the documented use case does not work if more than one slave node is used and if one slave node is used you might as well use <load_from_slaves>. Are there any other use cases I'm not aware of?

colinmollenhour commented 5 years ago

Yes it sounds like the documentation should be removed and perhaps a notice that it was removed and why.. Proper sharding would require a lot more work and I seriously doubt that anyone truly needs that, rather they would just want load balanced replicas like you.

gabi77 commented 5 years ago

Hi, I use 3 nodes 1 master 2 slave at Elasticache But the problem exists.

Even creating a server redis cluster with docker. We have a problem with local hash md5. We are currently looking to solve this problem

thanks,

colinmollenhour commented 5 years ago

Please look at PR #138 which changes the cluster config to just randomly select a slave, and adds support for multiple servers in the load_from_slave option.

gabi77 commented 5 years ago

Hi, The errors have been solved. not error MD5 on AWS elasticache

Thanks,

colinmollenhour commented 5 years ago

Thanks for testing, #138 has been merged and tagged as 1.11.0