Additional Session Prefix

[Genaker]

In big cloud environments, one Redis/keyDB instance used for many Magento installations. This prefix is necessary to properly manage sessions. Thank you for merging these changes

[Genaker]

already fixed

[colinmollenhour]

The non-use of the SESSION_PREFIX constant was actually intentional because I experienced odd cases where during shutdown the class constants had already been unloaded by PHP and then it reported an error that it was not defined or something strange like that..

I strongly recommend against sharing one Redis instance between Magento instances. The problem is that with shared resources one instance could affect all of the other instances negatively, for example by exhausting available memory and causing evictions for other clients. Also, you will actually get better performance using one Redis instance per Magneto installation since Redis is single-threaded and the Magento instances are excellent and natural "sharding" points.

With Docker it is so incredibly simple to setup additional redis servers. E.g. if using docker-compose:

services:
  redis-session:
    image: redis:5-alpine
    command: redis-server --appendonly yes
    volumes:
      sessions:/data
volumes:
  sessions:

Or if not yet using Docker you can still easily do it:

$ docker run -p 127.0.0.1:63790:6379 --name redis-session_1 -d -v redis-session_1:/data redis:5-alpine redis-server --appendonly yes
$ docker run -p 127.0.0.1:63791:6379 --name redis-session_2 -d -v redis-session_2:/data redis:5-alpine redis-server --appendonly yes
$ docker run -p 127.0.0.1:63792:6379 --name redis-session_3 -d -v redis-session_3:/data redis:5-alpine redis-server --appendonly yes
$ docker run -p 127.0.0.1:63793:6379 --name redis-session_4 -d -v redis-session_4:/data redis:5-alpine redis-server --appendonly yes

The overhead for running separate instances is so miniscule it is basically negligible (~2MB).

Another advantage is you can set different memory limits for each instance and also easily move one dataset to a different server without affecting the others.

Lastly, the longer your prefix the more memory your indexes will consume.

[Genaker]

You are right, but this parameter will be configurable if somebody doesn't want use he will not use it. We can't run docker Redis because we using AWS ELASTICCACHE and we need just this parameter logically separate sessions keys, we don't care about the performance of the session because it is jus single request per page and we need it for development environment - to have shared session for 1-20 development environments managed by aws elastic beanstalk. This code doesn't affect the existing user's performance. I can move this Const (SESSION_PREFIX and PREFIS_ADDITIONAL) to the class property (this->prefix) to avoid constant issues.

[colinmollenhour]

I see, thanks for the additional info.

A couple other thoughts:

• Due to the session cookies being random you are already extremely unlikely to get collisions even without a prefix.
  • Tuning PHP's session generation parameters to ensure sufficient randomness is a good idea either way.
• The "database" parameter is already supported so if each instance used a different database number this would be like a prefix but without increasing the size of your indexes and also allowing for better manageability (e.g. you can drop individual databases).

If you change the constant back I'll consider merging it, but it still seems unnecessary to me.

[Genaker]

The problem is not random cookie, Session key it is not human-readable.
DB instance ID is not human-readable.
After environment delation, we also want to remove all garbage. We need to remove the cookies associated with that environment (most of the environments are temporary). With DB ID we can't do that because we can't generate a proper id for each environment we need to build an additional system to keep an association environment with DB ID add keep documentation and teach new developers how to associate env with a new ID. In the case in the prefix, we will have a human-readable session key like sesion_staging_$sessionID. Also, AWS Redis by default has 16 DB limits.

[Genaker]

Session constant removed.

[Genaker]

Py the way it doesn't work with Magento because the configuration doesn't work like the dataObject and it doesn't have this method getPrefix();

[Genaker]

What do you think about adding prefix as a configurable parameter this->config->getPrefix() with a default value 'sess_'
without getAdditionalPrefix()? Then if somebody wants override can override the default parameter.

[colinmollenhour]

Sounds good as long as it is fully BC.

[joshua-bn]

Use another ElastiCache instance? This promotes a bad practice IMO.

[Genaker]

Where you can see bad practices there? One instance per different keys is ok. Different instances produce price and management overhead. Especially if you are building something big like SaaS or Cloud one Redis is better 10.