Create submodule to store PHP sessions in Redis

I am deeply interested in this and was going to implement something similar myself. I will gladly try it to see how it works and help on completing it. It looks promising, and you may very well saved me a lot of time...

So something on the lines of what https://www.drupal.org/project/session_proxy did on D7?

I was thinking about something similar, although I'm still figuring out how SessionManager and SessionHandler works (I'm not sure what is SessionHandler being used for).

I'm configuring redis/redis_sessions right now, so I will be able to tell you something later today.

Update: It is working good as far as I have tested for basic functionality (login/logout, maintaing session for awhile, storing values in $_SESSION, etc...).

I will develop some tests in the coming days, including stress tests, as I progress on my project to see if I detect some strange behavior.

Good work!

I could use this on Drupal7. I am migrating from MongoDB to Redis. I will see if I have time to backport this.

Thanks. A test for this would be great, we can't execute them on drupal.org but we usually do that as merge requests then on https://github.com/md-systems/redis.

There is for example a full web test that enables all the redis services and does a few things, we could include the session module there.

@tobby is there a way to specify the redis database in the save_path?

I'm trying

$settings['redis_sessions'] = [
  'save_path' => 'tcp://user:password@host:port/database',
];

But I'm getting a

[Wed Jan 10 14:03:04.641040 2018] [php7:notice] [pid 85468] [client ::1:55261] Uncaught PHP Exception Predis\\Response\\ServerException: "ERR wrong number of arguments for 'set' command" at /Applications/XAMPP/xamppfiles/htdocs/drupal/vendor/predis/predis/src/Client.php line 370, referer: https://localhost/drupal/admin/modules

Yeah, this patch doesn't seems to be working on 8.x-1.0-rc2
At first I thought it was working, but after flushing the redis db, I was still logged in. Also, The error I mentioned above happening while trying to logout.

@andreperazzi Is Predis supported in the Drupal 8 version of this module? This module's frontpage leads me to believe that it isn't:

Only the redis php extension is currently supported

I have this patch up and running fine with phpredis.

1. The RedisSessionsSessionManager->getSavePath method only gets server IP from Redis module, the port is hardcoded.

#this line should be replace
$save_path = "tcp://${settings['host']}:6379";
#by
$save_path = "tcp://${settings['host']}:${settings['host']}";

2. I get the following errors with 8.5
Error: Call to undefined method Symfony\Component\HttpFoundation\Session\SessionBagProxy::all() in Drupal\redis_sessions\RedisSessionsSessionManager->getSessionBagUid() (line 172 of modules/contrib/redis/modules/redis_sessions/src/RedisSessionsSessionManager.php).
or
RuntimeException: Failed to start the session in Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage->start() (line 146 of /app/vendor/symfony/http-foundation/Session/Storage/NativeSessionStorage.php).

3. This module doesn't work with sentinel or cluster modes. Either add a method to ClientInterface to return the current Redis IP/Port or just stick to $settings['save_path']

I was encountering too many issues with the current patch. I addressed some of the feedback from #12. I think the behavior to disable saving anonymous user sessions should be configurable still.

This will definitely need test coverage.

\Drupal\Tests\redis\Functional\WebTest should already use it automatically as it relies on the example.services.yml. To run tests, someone needs to start a pulll request against https://github.com/md-systems/redis.

For other backends, we often extend the core test coverage e.g. \Drupal\Tests\redis\Kernel\RedisLockTest. I guess \Drupal\system\Tests\Session\SessionTest would be a candidate for this. There is at least one method that does a direct query against the session table, so at least that will need to be duplicated, but the others *might* work?

Why are you creating a new SessionManager? Wouldn't it be simpler to create a new SessionHandler and alter the service class for the core session_handler.storage to use the newly created RedisSessionHandler?

#13 patch + Fixing getSessionBagUid error

@mpastas, I just tried applying your patch to drupal/redis:1.2 on a site running drupal/core 8.6.15 and PHP 7.3.11 and I'm seeing this error now

app_1            | [19-Nov-2019 22:36:21 UTC] RuntimeException: Failed to start the session in /app/vendor/symfony/http-foundation/Session/Storage/NativeSessionStorage.php on line 150 #0 /app/docroot/core/lib/Drupal/Core/Session/SessionManager.php(164): Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage->start()
app_1            | #1 /app/docroot/core/lib/Drupal/Core/Session/SessionManager.php(118): Drupal\Core\Session\SessionManager->startNow()
app_1            | #2 /app/vendor/symfony/http-foundation/Session/Session.php(57): Drupal\Core\Session\SessionManager->start()
app_1            | #3 /app/docroot/core/lib/Drupal/Core/StackMiddleware/Session.php(53): Symfony\Component\HttpFoundation\Session\Session->start()
app_1            | #4 /app/docroot/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
app_1            | #5 /app/docroot/core/modules/page_cache/src/StackMiddleware/PageCache.php(99): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
app_1            | #6 /app/docroot/core/modules/page_cache/src/StackMiddleware/PageCache.php(78): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
app_1            | #7 /app/docroot/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
app_1            | #8 /app/docroot/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
app_1            | #9 /app/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
app_1            | #10 /app/docroot/core/lib/Drupal/Core/DrupalKernel.php(693): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
app_1            | #11 /app/docroot/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
app_1            | #12 {main}

Same setup but with PHP 7.2. and it runs just fine
`

Great job but i have problem with bigpipe :
(D 8.8.1 , php 7.2.24)

The website encountered an unexpected error. Please try again later.
RuntimeException: Failed to start the session because headers have already been sent by "/app/drupal/web/core/modules/big_pipe/src/Render/BigPipe.php" at line 264. in Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage->start() (line 145 of /app/drupal/vendor/symfony/http-foundation/Session/Storage/NativeSessionStorage.php).

Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage->start() (Line: 164)
Drupal\Core\Session\SessionManager->startNow() (Line: 195)
Drupal\Core\Session\SessionManager->save() (Line: 217)
Drupal\redis_sessions\RedisSessionsSessionManager->save() (Line: 196)
Symfony\Component\HttpFoundation\Session\Session->save() (Line: 248)
Drupal\big_pipe\Render\BigPipe->performPostSendTasks() (Line: 308)
Drupal\big_pipe\Render\BigPipe->sendContent(Object) (Line: 112)
Drupal\big_pipe\Render\BigPipeResponse->sendContent() (Line: 374)
Symfony\Component\HttpFoundation\Response->send() (Line: 20)

1. +++ b/example.services.yml
   @@ -31,3 +31,10 @@ services:
   +  # Replaces the session manager with a redis implementation.
   +  session_manager:
   +    class: Drupal\redis\Session\SessionManager
   +    arguments: ['@request_stack', '@redis.factory', '@session_manager.metadata_bag', '@session_configuration', '@session_handler']
   +    calls:
   +      - [setWriteSafeHandler, ['@session_handler.write_safe']]
   

   This seems broken if it exists in the submodule, which decorates the parent service.

2. +++ b/modules/redis_sessions/redis_sessions.info.yml
   @@ -0,0 +1,8 @@
   +version: 1.0
   

   Remove this, added by packager

3. +++ b/modules/redis_sessions/redis_sessions.info.yml
   @@ -0,0 +1,8 @@
   +core: 8.x
   

   Reroll for D9

4. +++ b/modules/redis_sessions/src/RedisSessionsSessionManager.php
   @@ -0,0 +1,302 @@
   +  public function __call($method, $args) {
   +    return call_user_func_array(array($this->innerService, $method), $args);
   +  }
   

   This isn't needed since you're extending the class directly, not just implementing the interface.

5. +++ b/modules/redis_sessions/src/RedisSessionsSessionManager.php
   @@ -0,0 +1,302 @@
   +    if ($this->isCli()) {
   +      // We don't have anything to do if we are not allowed to save the session.
   +      return;
   +    }
   +
   +    if ($this->isSessionObsolete()) {
   +      // There is no session data to store, destroy the session if it was
   +      // previously started.
   +      if ($this->getSaveHandler()->isActive()) {
   +        $this->destroy();
   +      }
   +    }
   +    else {
   +      // There is session data to store. Start the session if it is not already
   +      // started.
   +      if (!$this->getSaveHandler()->isActive()) {
   +        $this->startNow();
   +      }
   +      // Write the session data.
   +      $this->innerService->save();
   +    }
   +
   +    $this->startedLazy = FALSE;
   

   This could be replaced with a parent::save call.

Also, couldn't we find a way to leverage \Symfony\Component\HttpFoundation\Session\Storage\Handler\RedisSessionHandler introduced in Symfony 4.1?

In the case of using clustered Redis (PhpRedisCluster), there are some changes required, check this for details.

I have attached a refreshed patch that provides basic support for the clustered Redis.

I have rerolled patch and applied some adjustments from https://www.drupal.org/project/redis/issues/2876099#comment-13946294.

As I can see TTL for users sessions is not set (-1). Probably it should be fixed by something like this:

$this->redis->setEx($this->getUidSessionKey(), (int) ini_get('session.gc_maxlifetime'), $this->getKey());

instead of

$this->redis->set($this->getUidSessionKey(), $this->getKey());

Hello,

I have created a MR for easier code review.

I have not tested the patch as I intended to do initially because I don't understand why there is a new class in the main Redis module and almost the same class in a submodule. Which service should be used?

Hello,

I have tested the class which was in the main module as its code seemed more elaborated. I put it in the submodules, cleaned up dependency injection.

I searched to leverage \Symfony\Component\HttpFoundation\Session\Storage\Handler\RedisSessionHandle as mentionned in comment 20. I will try to make the change tomorrow.

Still a lot of code I wonder why it here.

Also, I wonder if two classes should not be made to distingish phpredis and predis, and a factory introduced such as in the main Redis module.

Hello,

I have updated the MR to:
- use Symfony\Component\HttpFoundation\Session\Storage\Handler\RedisSessionHandler as mentioned in comment 20
- only override the Session storage as mentioned in comment 15

Which results in a simpler change.

I have currently removed the settings.php config, as it uses the one used for the cache.

I think it should be possible to declare a dedicated redis connection for sessions, but this would mean to refactor ClientFactory if I am not mistaking.

And it still needs tests. I will try to give a look at that.

Thanks for the review.

Test added.

I will open a PR on https://github.com/md-systems/redis

Yay tests. FWIW, I don't think tests on github are still working, I don't think that thing we use there is still available? If someone wants to integrate something else that is still available for free for open source projects that would be great, but I won't have time to do that myself I think.

PR created https://github.com/md-systems/redis/pull/33.

Thanks for the review!

Tests does not seem to be triggered...

PS: lol cross posting :)

And tests on drupal.org, customizing drupalci.yml to install a redis?

The problem may be the matrix of tests between phpreid and predis.

All the tests to update...

Sorry for spamming.

Is it only that the github repository needs to use https://www.travis-ci.com/ and no more https://travis-ci.org/?

Also I wonder, regarding the change size, if this could not be included in the Redis module directly, so no need to create a sub-module.

I don't think we can install a redis server or php extension through drupalci commands.

And no, I don't think it's just changing to .com. .org was the free service for open source projects, AFAIK that is no longer provided at all.

To have been able to install java, go https://git.drupalcode.org/project/file_extractor/-/blob/4.0.x/drupalci.yml

Or Docker compose https://git.drupalcode.org/project/cmis/-/blob/3.0.x/drupalci.yml

I think it is possible to have a redis server running on drupalci.

On https://www.travis-ci.com/ it is written "Testing your open source projects is always 100% free!" but as I am not using it on a daily basis I don't know if some changes happened.

This looks really good, nice simple implementation using existing components.

As far as code review goes I have one question about class naming: is this a factory or a proxy? It feels like a proxy to me but I'm very happy to be wrong.

I would personally like to see this simply included in the main redis module, I see no reason to cordon off sessions as a special case, but again, happy to be outvoted/wrong about that.

I couldn't use the patch generated by GitLab so if anyone else is looking to try this out, here's a patch against 8.x-1.x from the current state of the MR. No new work in this patch.

Tested out the patch, works beautifully, thank you!

So I found a potential problem, in short: I keep getting logged out.

Turns out standard Drupal is pretty bad at session garbage collection, but redis is excellent, which is one of the reasons I wanted to use it in the first place. A bit of testing revealed to me (you all might already know these):

• default.services.yml lists the default value for session.gc_maxlifetime as 200000 seconds or about 2.3 days.
• If you don't explicitly set session.gc_maxlifetime in your services.yml file, it actually defaults to 24 minutes, at least on the docker image I'm using.
• Drupal doesn't clear the session records in the database after that period, in fact I can see some really old sessions in that table.
• Redis on the other hand, won't return a value that has expired, it seems.

This led to a situation where I thought Drupal had some defaults in place that resulted in long session lifetimes and I expected that to remain true when I switched to redis as the backend. It didn't.

This could be solved by documentation of course. I wonder if it's reason enough to leave this in a separate submodule so that it's a conscious choice to include it and configure it properly.

Added a bit of info about session lifetimes to the readme, can't hurt.

This works well for me, marking as RTBC.

Only issue I noticed was if $settings['session_write_interval'] > gc_maxlifetime your sessions will always expire. Not that anybody will realistically run into this. I only did because I was testing writes.

Working well for me too, tested in the scope of #3267531: Error in WriteSafeSessionHandler when switching to read only DB user

I am using the redis module for drupal login session management in my project using this article (https://stackoverflow.com/questions/41486546/drupal-8-overriding-session...). Now what happens is if the same user login from multiple browsers it creates multiple sessions for the same user. But the requirement is only one active session will be allowed at a time for the same user id. If the user has an ongoing session in any other browser or system, upon logging in to another, will be logged out of the previous active session.

Can anyone help on this issue?

@B N Pandey what you describe sounds like it's outside the scope of this project. A single session per user is not the default way PHP or Drupal sessions work.

Thanks for your update @pianomansam. Is there any way to achieve this functionality, any code or reference using redis?

@B N Pandey redis is a storage engine, it stores whatever you put in it, so no, there is no redis-specific way to achieve what you're asking.

Sessions are not the way to keep track of users across devices, it just can't be done that way.

If you need to store user-specific data for logged in users maybe have a look at https://api.drupal.org/api/drupal/core%21modules%21user%21src%21UserData...

If you want to track an anonymous user across multiple devices you'll need some kind of third party identification system. Pretty sure that kind of thing would exist, also pretty sure it would violate a bunch of privacy principles.

Forced rebase against dev branch. Here's a patch if anyone needs it.

Added a commit to make the submodule compatible with Drupal 10

Merge request is failing.

Ah ok, it wasn't that simple then, I'll do a more thorough job, thanks for pointing it out :)

You can easily override the Drupal session factory via *services.yml to the Redis session factory (a component that Symfony provides).

services:
  redis.client:
    class: Redis # Your Redis client class.
    factory: [ '@redis.factory', 'getClient' ]
  session_handler.storage:
    class: Symfony\Component\HttpFoundation\Session\Storage\Handler\RedisSessionHandler
    arguments: [ '@redis.client', { prefix: 'drupal_session:' } ]

I'm not sure if there are any downsides to this approach. If not, then mentioning this feature in the README would be helpful.

@kksandr

How your redis.client class looks like?

@hmdnawaz I use phpredis, so it's class: Redis, which is the global class provided by that PHP extension.

In case of Drupal redis module, can we use?

services:
  redis.client:
    class: Drupal\redis\Client\PhpRedis # Your Redis client class.
    factory: [ '@redis.factory', 'getClient' ]
  session_handler.storage:
    class: Symfony\Component\HttpFoundation\Session\Storage\Handler\RedisSessionHandler
    arguments: [ '@redis.client', { prefix: 'drupal_session:' } ]

#52: works for me ;) thanks!

There is an issue when using the RedisSessionHandler. When you clear the cache from the UI or use Drush, the logged-in user gets logged out.

#52: works for me ;)
just adding steps

in your custom module add

services:
  redis.client:
    class: Drupal\redis\Client\PhpRedis
    factory: [ '@redis.factory', 'getClient' ]

  session_handler.storage:
    class: Symfony\Component\HttpFoundation\Session\Storage\Handler\RedisSessionHandler
    arguments: [ '@redis.client', { prefix: 'drupal_session:' } ]

and in settings.php add :
$settings['session_handler_class'] = 'Symfony\Component\HttpFoundation\Session\Storage\Handler\RedisSessionHandler';

hope this help to others :)

I propose we adjust this issue to fully document that approach in the module code and docs.

I'm not sure what exactly is the "correct" way to do this... on a recent project we simply did $settings['session_storage']['class'] = 'Drupal\redis\Cache\PhpRedis'; and it seemed to work pretty well! if anyone can provide a comparison or justification on the best approach, I would much appreciate it.

In general, for our project, which consists of mostly logged in users, we saw a ~600% increase in throughput under load between that setting and also adding $settings['session_write_lock'] = FALSE;. Managing sessions through Redis appears to be a boon for performance and it would be great if this got rolled into the "default" config and setup.

YMMV of course, as there are so many factors to consider.

FYI as far as I can tell the $settings variable overrides above do nothing in Drupal 11 and the only way to swap the session handler is via overriding the services in services.yml.

Thank you for working on this.
As @berdir now have open 2.x and writes that, no new features will be added to 8.x-1.x. Should perhaps this feature be built on top of the 2.x branch?

Hi,
As I understand Redis the LRU eviction policy is a global server setting (all keys and db). So if you setup sessions and cache on the same instance, sessions could be delete if memory is full.

Did you find a way to manage this side effect?