perf: php session is always written because of __MTIME value

[tpayen]

On each request the session __MTIME variable is updated with the time() value so the session is written on each request.
It can be a performance issue when using shared session handler as Redis, as we can see a lot of network traffic to update all this data.
To reduce the network consumption we choose to update __MTIME only a time to time, and a little arbitrary we choose to do it each 1/10 session lifetime. As __MTIME acted probably as a keep alive for the session, it's steel a good idea to update it regulary, but not too regulary

In our infrastructure, this change reduce the network consumption of Redis by half, so I think it worth the change

[alecpl]

As far as I can see the $changed property is not used in php session handler. It has it's purpose in other handlers. I think anything referencing $changed and __MTIME can be safely removed from the rcube_session_php code.

ps. why don't you use the redis session handler?

[tpayen]

Yeah ok I thought the $changed property wasn't being used. If I update the PR to remove use of $changed and __MTIME in rcube_session_php it's ok for you ?

ps. why don't you use the redis session handler?

We are using a Redis backend as session handler, so we don't have choice ;-) But we also have lock session issues with redis so we are thinking to change maybe for redis sentinel

[alecpl]

Yeah ok I thought the $changed property wasn't being used. If I update the PR to remove use of $changed and __MTIME in rcube_session_php it's ok for you ?

Yes, I suppose.

ps. why don't you use the redis session handler?

We are using a Redis backend as session handler, so we don't have choice ;-)

$config['session_storage'] can be set to 'redis', but it seems you use 'php'.

[tpayen]

$config['session_storage'] can be set to 'redis', but it seems you use 'php'.

Yes, we use 'php' as 'session_storage' with session_handler configured with 'RedisCluster' 'cause we use a redis cluster (multiple masters) and it's not supported in roundcube :

        // only allow 1 host for now until we support clustering
        if (count($hosts) > 1) {
            rcube::raise_error([
                'code' => 604,
                'type' => 'redis',
                'line' => __LINE__,
                'file' => __FILE__,
                'message' => 'Redis cluster not yet supported',
            ], true, true);
        }

But as RedisCluster doesn't support session locking, this isn't the best option. If we go to Redis Sentinel, I'll probably make a PR to support sentinel in roundcube (the client needs to know the sentinels).

We need clustering as we have 100k-150k very actives users