Ability to connect to Redis Sentinel for Cache, PageCache and Session

[romainruaud]

Description

This PR provides the support of Redis Sentinel for Cache, PageCache, and Session.

Usage of Redis Sentinel is suitable in an HA environment.

This is supported "out of the box" by colinmollenhour modules (credis, php-abstract-session, and it's cache backend). See https://github.com/colinmollenhour/Cm_Cache_Backend_Redis#high-availability-and-load-balancing-support

But Magento is not actually fully compatible with Redis Sentinel usage. This PR intends to add this compatibility.

What has been done :

• leveraging dependencies to colinmollenhour modules.
• adding the ability to support additional configuration variables related to Redis Sentinel when installing with command line.
• updating unit tests to remain compliant.

New variables added to the setup:install in command line :

--cache-backend-redis-sentinel-master
--cache-backend-redis-sentinel-master-verify
--cache-backend-redis-sentinel-load-from-slaves
--page-cache-redis-sentinel-master
--page-cache-redis-sentinel-master-verify
--page-cache-redis-sentinel-load-from-slaves
--session-save-redis-sentinel-master
--session-save-redis-sentinel-master-verify
--session-save-redis-sentinel-load-from-slaves
--session-save-redis-sentinel-connect-retries

Basically, there are three parameters that are identical in the 3 cases :

Argument	Required ?	Values	Comment
--cache-backend-redis-sentinel-master	No	string	the name of the Redis master which is monitored by the sentinel servers.
--cache-backend-redis-sentinel-master-verify	No	0/1	Verify connected server is actually master as per Sentinel client spec.
--cache-backend-redis-sentinel-load-from-slaves	No	1/2	A random slave will be chosen for performing reads in order to load balance across multiple Redis instances. Using the value '1' indicates to only load from slaves and '2' to include the master in the random read slave selection.
--page-cache-redis-sentinel-master	No	string	the name of the Redis master which is monitored by the sentinel servers.
--page-cache-redis-sentinel-master-verify	No	0/1	Verify connected server is actually master as per Sentinel client spec.
--page-cache-redis-sentinel-load-from-slaves	No	1/2	A random slave will be chosen for performing reads in order to load balance across multiple Redis instances. Using the value '1' indicates to only load from slaves and '2' to include the master in the random read slave selection.
--session-save-redis-sentinel-master	No	string	the name of the Redis master which is monitored by the sentinel servers.
--session-save-redis-sentinel-master-verify	No	0/1	Verify connected server is actually master as per Sentinel client spec.
--session-save-redis-sentinel-load-from-slaves	No	1/2	A random slave will be chosen for performing reads in order to load balance across multiple Redis instances. Using the value '1' indicates to only load from slaves and '2' to include the master in the random read slave selection.
--session-save-redis-sentinel-connect-retries	No	string	Number of connect retries when contacting sentinels

Please also note that when using Sentinel, you must use the IP/address of your sentinel servers as your Redis Host. Since you are meant to have several sentinels when running such an instance, you can use the list of all your sentinel servers as an input for the "host" or "server" configuration values for Redis (Eg : 'tcp://10.0.3.1:26379,tcp://10.0.3.2:26379,tcp://10.0.3.3:26379') .

Here is an example of a generated app/etc/env.php :

  'session' => 
  array (
    'save' => 'redis',
    'redis' => 
    array (
      'host' => 'tcp://10.0.3.1:26379,tcp://10.0.3.2:26379,tcp://10.0.3.3:26379', // Sentinel servers
      'port' => '26379',
      'password' => '',
      'timeout' => '2.5',
      'persistent_identifier' => '',
      'database' => '0',
      'compression_threshold' => '2048',
      'compression_library' => 'gzip',
      'log_level' => '1',
      'max_concurrency' => '6',
      'break_after_frontend' => '5',
      'break_after_adminhtml' => '30',
      'fail_after_frontend' => '1',
      'fail_after_adminhtml' => '10',
      'first_lifetime' => '600',
      'bot_first_lifetime' => '60',
      'bot_lifetime' => '7200',
      'disable_locking' => '0',
      'min_lifetime' => '60',
      'max_lifetime' => '2592000',
      'sentinel_master' => 'redismaster', // Name of master monitored by Sentinels.
      'sentinel_master_verify' => '1',
      'load_from_slaves' => '2',
      'sentinel_connect_retries' => '3',
    ),
  ),
  'cache' => 
  array (
    'frontend' => 
    array (
      'default' => 
      array (
        'backend' => 'Cm_Cache_Backend_Redis',
        'backend_options' => 
        array (
          'server' => 'tcp://10.0.3.1:26379,tcp://10.0.3.2:26379,tcp://10.0.3.3:26379', // Sentinel servers
          'database' => '1',
          'port' => '26379',
          'sentinel_master' => 'redismaster', // Name of master monitored by Sentinels.
          'sentinel_master_verify' => '1',
          'load_from_slaves' => '2',
        ),
      ),
      'page_cache' => 
      array (
        'backend' => 'Cm_Cache_Backend_Redis',
        'backend_options' => 
        array (
          'server' => 'tcp://10.0.3.1:26379,tcp://10.0.3.2:26379,tcp://10.0.3.3:26379', // Sentinel servers
          'database' => '2',
          'port' => '26379',
          'compress_data' => '0',
          'sentinel_master' => 'redismaster', // Name of master monitored by Sentinels.
          'sentinel_master_verify' => '1',
          'load_from_slaves' => '2',
        ),
      ),
    ),
  ),

Manual testing scenarios

1. ... Install a Redis Sentinel Cluster somewhere (there are some "fast to setup" examples with Docker-Compose)
2. ... Install Magento2 with command line and specify these set of parameters :

	--cache-backend=redis \
        --cache-backend-redis-server=tcp://10.0.3.1 \
        --cache-backend-redis-db=1 \
        --cache-backend-redis-port=26379 \
	--cache-backend-redis-sentinel-master=redismaster \
	--cache-backend-redis-sentinel-master-verify=1 \
	--cache-backend-redis-sentinel-load-from-slaves=2 \
        --page-cache=redis \
        --page-cache-redis-server=tcp://10.0.3.1:26379 \
        --page-cache-redis-db=2 \
        --page-cache-redis-port=26379 \
	--page-cache-redis-sentinel-master=redismaster \
	--page-cache-redis-sentinel-master-verify=1 \
	--page-cache-redis-sentinel-load-from-slaves=2 \
        --session-save=redis \
        --session-save-redis-host=tcp://10.0.3.1:26379 \
        --session-save-redis-db=0 \
	--session-save-redis-port=26379 \
	--session-save-redis-host=tcp://10.0.3.1:26379 \
	--session-save-redis-sentinel-master=redismaster \
	--session-save-redis-sentinel-master-verify=1 \
	--session-save-redis-sentinel-load-from-slaves=2 \
	--session-save-redis-sentinel-connect-retries=3 \

3. Check everything is running properly (cache keys are here in Redis, the same for page cache and sessions).

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[romainruaud]

I fixed PHPCs checks.

[orlangur]

Please change to

+        "colinmollenhour/credis": "~1.9.0",
 +        "colinmollenhour/php-redis-session-abstract": "~1.3.5",

[romainruaud]

Ok, will do it asap. Regards

[orlangur]

@vrann note that composer.* changes needs to be applied to EE/B2B codebase to be properly tested on Bamboo.

[romainruaud]

@orlangur I updated composer.json as you said.

Regards

[orlangur]

composer.lock needs to be updated too, but let's wait for formal review from @vrann. It can be done in the very end before squashing everything into one clean commit.

[romainruaud]

@orlangur is any help from me needed for this issue ? Especially regarding conflicts. Or are you guys actually working internally on this one to test it and merge it by your own ?

Best regards

[orlangur]

Hi @romainruaud, please resolve conflicts and force push changes as a single commit.

I'll try to find somebody familiar with Redis Sentinel to be able to play with your implementation and review it.

[romainruaud]

@orlangur I fixed the conflicts, rebased the branch and squashed the commits.

Let me know if more help is needed.

Best regards

[orlangur]

@vrann agreed to look into it as he even wrote some HLD involving Redis Sentinel.
@romainruaud, thanks for your patience.

[romainruaud]

Great to see you are gonna have a look on this one, actually Redis is the last component which is not HA-friendly and prevent this prevent us (but anybody else) to build complete autoscaling platform.

@orlangur I see that Travis is broken due to composer.lock file not being up-to-date. Do you want me to fix this on this branch or will you process the fix by yourselves when integrating it to develop ?

Regards

[orlangur]

@romainruaud please fix and do and amend commit + force push.

[metal3d]

It's been a while that I wonder why Magento has not implemented sentinels usage and that's a real problem if we want to get Redis Hight Available.

I am impatient to finally be able to avoid losing sessions if Redis craches...

Is there any chance to see that PR to be implemented soon ?

[kandy]

@metal3d, looks like the support of Redis sentinels already available in 2.2 develop and this PR only add the possibility to configure it in install step. But I personally does not see reason why we need to do it in install and not after

[romainruaud]

@kandy "But I personally does not see reason why we need to do it in install and not after"

In fact we need it because we have a devops stack which is building environments from scratch (from a git repo) and processes the setup:install step if it's a totally new environment.

Also, this is not logical to have configuration options such as redis sentinel that we can only specify after installing. The install parameters should allow exactly the same thing than editing the env.php manually (since the installer is basically a php process which writes the env.php).

[kandy]

@romainruaud, Magento use bin/magento setup:config:set command for this

[romainruaud]

Sorry @kandy but I cannot agree. All other settings of env.file are available through setup:install.

There is not point doing setup:install with files as cache backend (and session) and process a setup:config:set just after.

=> I would lose the prebuilt cache.

For me the command should be consistant with the rest

[kierenevans]

@kandy Usage of HA redis for sessions would need at least v1.3.5 of colinmollenhour/php-redis-session-abstract, but 2.2-develop still ships with v1.3.4

[okorshenko]

Hi all
After the internal discussion, we decided to reject this PR. Please consider @kandy solution.

[metal3d]

Magento installation on command line needs to contact Redis, there is an error if we use Sentinels, that PR fixes that... and you refuse that fix.

Can you please explain the reason you don't want to fix this ? "Internal discussion" is not very informative.

Thanks.

[orlangur]

Hi @metal3d,

Magento installation on command line needs to contact Redis, there is an error if we use Sentinels, that PR fixes that

Please report an issue describing such scenario, then possible approaches to solve this problem can be discussed. Cannot say anything about internal discussion as didn't participate / read meeting minutes. "All other settings of env.file are available through setup:install" is a strong argument to me personally though if it is true.

[tschirmer]

@kandy Which cli command are you referring to; setup:config:set doesn't include those options to write to the env file appropriately?

The setup config doesn't have this option

 bin/magento setup:config:set --help

...
      --cache-backend=CACHE-BACKEND                                                              Default cache handler
      --cache-backend-redis-server=CACHE-BACKEND-REDIS-SERVER                                    Redis server
      --cache-backend-redis-db=CACHE-BACKEND-REDIS-DB                                            Database number for the cache
      --cache-backend-redis-port=CACHE-BACKEND-REDIS-PORT                                        Redis server listen port
      --cache-backend-redis-password=CACHE-BACKEND-REDIS-PASSWORD                                Redis server password
      --cache-backend-redis-compress-data=CACHE-BACKEND-REDIS-COMPRESS-DATA                      Set to 0 to disable compression (default is 1, enabled)
      --cache-backend-redis-compression-lib=CACHE-BACKEND-REDIS-COMPRESSION-LIB                  Compression lib to use [snappy,lzf,l4z,zstd,gzip] (leave blank to determine automatically)
      --cache-id-prefix=CACHE-ID-PREFIX                                                          ID prefix for cache keys
      --page-cache=PAGE-CACHE                                                                    Default cache handler
      --page-cache-redis-server=PAGE-CACHE-REDIS-SERVER                                          Redis server
      --page-cache-redis-db=PAGE-CACHE-REDIS-DB                                                  Database number for the cache
      --page-cache-redis-port=PAGE-CACHE-REDIS-PORT                                              Redis server listen port
      --page-cache-redis-password=PAGE-CACHE-REDIS-PASSWORD                                      Redis server password
      --page-cache-redis-compress-data=PAGE-CACHE-REDIS-COMPRESS-DATA                            Set to 1 to compress the full page cache (use 0 to disable)
      --page-cache-redis-compression-lib=PAGE-CACHE-REDIS-COMPRESSION-LIB                        Compression library to use [snappy,lzf,l4z,zstd,gzip] (leave blank to determine automatically)
...

[kandy]

@tschirmer @romainruaud Yep, I agree that is valuable to deliver it in 2.3/2.4 branch