matomo-org / plugin-QueuedTracking

Scale your large traffic Matomo service by queuing tracking requests (in Redis or MySQL) for better performance.
https://matomo.org
GNU General Public License v3.0
82 stars 35 forks source link

Use \RedisSentinel instead of Credis #146

Open eschultz-magix opened 3 years ago

eschultz-magix commented 3 years ago

The usages of a Redis library are very inconsistent.

Having 1 Redis server without sentinel --> phpredis with \Redis Having X Redis server in a cluster --> not supported Having X Redis server with sentinel --> Credis in standalone mode

In Sentinel mode, one of the Sentinel instances are picked, getMasterAddrByName() is called and then a connection is established to the master where all all/write operations are done like in normal Redis mode.

Credis uses phpredis by default if available, but phpredis also has a \RedisSentinel class which has the getMasterAddrByName() function.

TL;DR: Credis is only used in Sentinel mode and can get replaced by phpredis, which is required by QueuedTracking anyway.

tsteur commented 3 years ago

Thanks for creating the issue @eschultz-magix I'm not so much into the topic right now but I think a while back sentinel mode wouldn't work with phpredis. This might have changed.

tsteur commented 3 years ago

@eschultz-magix would there be any particular advantage of using one or the other?

eschultz-magix commented 3 years ago

@tsteur I guess, it would be great not to mix them up. Matomo requires the phpredis extension to be installed if you want to use redis as backend cache. Credis always checks if the extension is installed and fallbacks to phpredis. So there are no advantages, instead there is an additional check + library. In Sentinel model Credis forces its own implementation to communicate with the Sentinel server and with the Redis master server.

So at the end there are two different ways to communicate with Redis: phpredis (Matomo and QueuedTracking in Non-Sentinel mode) and Credis (QueuedTracking in Sentinel mode).

Using Credis will only make sense if you want to support old phpredis extensions without the \RedisSentinel class.

eschultz-magix commented 3 years ago

@tsteur The RedisSentinel class is available since phpredis 5.2.0

tsteur commented 3 years ago

Thanks for this @eschultz-magix didn't know RedisSentinel existed. We could prefer that one over Credis which we'd ideally still keep as a backup to not break backwards compatibility as not everyone can install extensions although in Matomo 5 we could think about removing Credis and fall back to MySQL instead although there be no harm to still keep supporting Credis I suppose besides having to keep dependency up to date.