Port Redis to Drupal 8

The sandbox looks like an awesome start! :-)

Nice, thanks! My opinion is that for D8 this module should get a full rewrite. FYI I wrote the original code in less than 2 hours, it evolved with advanced lifetime settings and a few bugfixes, but it's still a very small module. I'd try a review when I'll have some spare time, but as of now, I'm off for at least 2 weeks so I can't.

Hi everyone!

This project is one of my favorite modules in Drupal, thanks for your awesome work.

I propose separate this module in multiples sub-modules
* [1] Redis module: contains a service to connect on Redis (predis/phpredis)
* [2] Redis UI module: Contains a interface to configurate Redis module
* [3] CacheBackEnd module: provide a cachebackend use Redis
* [4] The composer autoload: The Redis module should use composer to load libraries
* [5] Use github to manage PR to develop the new version?
* [crazy idea] Only support to Predis, because you only need clone the repo or use composer o dowload a zip. I am not convinced of this but want to know what you think

[1] Redis should be a service because others module might need this, for example I have one module and I am interested in using Redis.

Regards!

Yes you're right, Redis should be a service, but there is no use of splitting the module into smaller ones, especially not with Drupal 8 since everything will be into classes in a well organized folder (yay PSR-0/4). We don't need to split everything, we just need to document what components it provides and how to use it.

I do not agree we should only support Predis, in fact I think we should continue to provide both PhpRedis and Predis (and BTW I saw more people interested or using PhpRedis than Predis, but that's what I saw, not what's true) -- keeping PhpRedis support is really important.

But I think we should separate both Predis and PhpRedis by providing different cache and lock backends: I think that merging both in the Drupal 7 module was an error and the module should provide 2 different services / backends instead of trying to mutualize both of them.

The redis service could be use a DI to create a Redis service object, each piece is replaceable

This depends if use Predis or Phpredis
$redis = (new Predis\Cliente() | new \Redis() );
$redis_service = Drupal\redis\Redis($redis);
$cache = new CacheBackend($redis_service);

Even thought it's logically the best choice, it's not in real life because API differs between Predis and PhpRedis, so it's better to separate the two and have to services in the end, I guess.

We can make wrappers for this to be transparent we can use interfaces to unify this clasess. Thinking that this library can be a standard connection to Redis in Drupal.

[*] http://en.wikipedia.org/wiki/Interface-based_programming

Already thought of this, already tried, but API is way too complete to do this, both evolve throught time, it does not worth the shot. It's easier to develop 2 different versions of the cache backend than maintain such wrapper, it's all a question of balance between maintainability and beauty. Best is the enemy of the good.

Redis is part of our recommended Drupal 8 hosting setup. We started integrating it into our custom Drupal 8 install profile based project with extensive test coverage, already months ago.

I do think the 8.x branch + dev release should be created soon in the d.o repo to allow the work to be visible in the regular 8.x module search. It allows better visibility for the module and this might result into more adoption. Redis is an awesome backend with its native tag support.

Sounds like a fair point. I don't like much writing 8.x code already even thought things are calming down it's still a moving target. But as far as I know, cache API is rather stable, so maybe that's the right moment to start an official 8.x branch.

I won't have much time until the second half of september, please don't hesitate to wake this issue when the time come.

Is the plan still to start working in an 8.x branch here, or is the sandbox still the place to focus efforts?

Currently, we are working with the sandbox of znerol and contributed through Berdir there.
So it's still the "official" sandbox and perfectly works for us.

We are happily open for testers, reviews, contributors and finally some merge path into the official project.

I still don't have a lot of time to handle the D8 version. znerol's sandbox sounds good, I just did a quick read, it's a port of actual code plus the various D8 additions (factories and config port, etc...) it should work seamlessly.

I'm sorry for not having that time, I will try whenever I will be able to do it.

I've rolled a patch over at #2376951: Keeping up with 8.

It currently has all PhpRedis-related tests passing. An open question is, do we keep the admin UI? I can't figure out a way to use \Drupal::config('redis.settings') that doesn't result in a cyclic recursion. I've updated the README a bit as well.

Short update here.

I've continued development a bit on my github repository, which has the advantage that we can actually run tests there with travis.

I've noticed a nasty bug with cache tag invalidations, and fixed that here, together with a few related refactorings: https://github.com/md-systems/redis/pull/1. Next up is removing the admin form and test for it, because it's not possible to configure those things in the UI anymore in 8.x, as they are settings, which should make travis green (with only a single test remaining for now).

Next on my list is refactoring the current static factory calls with services and actually injecting them, which would also allow for unit tests later on.

The phpredis implementation seems to be working now, but needs a lot of testing, also for performance. There's some feedback on it from @pounard in #2376951: Keeping up with 8, I can't comment much on that yet, to be honest, I learned most that I know about redis today while debugging the problem that I had. Just one thing for now:

You are correct that using those more complex methods prevents the use of sharding as it was possible in 7.x. We should be able at least keep different bins on different servers, but then we need to replicate the cache tag and stale lists then, because those simply are not bin specific, by design.

The only other option I see is to implement a similar API than core has, which means a cache tag checksum service that counts invalidations and compare them when loading a cache entry. We could offer that as a separate implementation, or even based on settings from the same cache backend factory. Those factories and services a great deal more flexibility than 7.x

If you use proxy assisted sharding, you just won't be able to command the proxy which keys need to be altogether, so this won't work. You can per-bin shard using another method than proxy assisted sharding but you probably will need to maintain a redis server list on the Drupal side in the end, and I think it's rather a bad idea, I'd prefer to leave my sysadmin choose for me how to shard and just give me one single IP. And honestly, it don't think a per-bin shard would do the trick, if you look at how the bins behave, in most sites you have one huge bin that would get alot of data and cause performance problems, and a lot of other bins which are small and don't cause any trouble. I don't think it's realistic to try this, it will make the code even more complex for something that I don't think would really solve any problems.

As we're getting nearer a D8 release candidate, and I'm working on finalizing some D8 infrastructure work, might it be a good time to work on moving the sandbox code into a proper 8.x-dev branch for this module? And is this sandbox (https://drupal.org/sandbox/znerol/2233417) still the location of the latest work?

The only other option I see is to implement a similar API than core has, which means a cache tag checksum service that counts invalidations and compare them when loading a cache entry.

This is actually a good idea! Database backend is not so optimized for the cache tag checksum implementation core has, but the exact very same in Redis would be very fast, and shardable. Please, implement the same using one KEY per tag, and use the INCR command, and you'll have a very fast and shardable implementation.

It could also store cache tag checksums in a HASH using HINCRBY, but this would be a bit less scalable because tag keys wouldn't be split over multiple instances but only stored in one single instance, but this shoud work the same as long as you don't have thousands of tags. But let's try to be pragmatic, you don't need performances if you don't have thousands of users or nodes, so you *will certainly* have tens of thousands of tags, so just use on key per tag as a first implementation, and benchmark this first use case with real datasets.

We could offer that as a separate implementation

This is actually good idea, but in the end, anyone using memcache will use a memcache implementation, anyone using mongo will use a mongo implementation, and whatsoever, just focus on Redis implementation as a first step and don't over engeneer!

D8 seems to be a bit over-engeneered to me, that's what makes it so hard to understand IMHO, I'd like to keep Redis connector simple and functionnal as a first step, we'll see later for making it a bite more invasive.

@geerlingguy They are working on a github repository as of now, please see upper comments.

@all FIY I am working on a 3.x version for D7 because 2.x as some design flaws that makes it harder to solve a few problems, and code duplication between PhpRedis and Predis is slowing me down a lot. V3 focuses on making consistency code backend agnostic, so I had to split the backend into 2 parts: one DrupalCacheInterface implementation with common code, and the other much more pragmatic and small which really implements Redis comments, so the module is actually going to radically change in the next few weeks.

Hello, please consider looking at the 3.0 new release (7.x-3.x branch) - I'm not sure all the work I did there will actually help for Drupal 8, since the cache interfaces have evolved so much, but code in this new branch should be clearer.

Updated the issue summary with links to both the sandbox and GH repo where this is being worked on. Thanks!

FYI, I've been testing the Redis 8.x-1.x dev release from the GitHub project linked in the issue summary on my Raspberry Pi-based Dramble, and so far have been able to verify that, with out of the box settings, Drupal 8 loads uncached pages 15% faster when simply using Redis as the default cache backend.

A pretty good speedup! Here are the detailed findings: Configure Redis for shared session caching, and k-v store.

I didn't encounter any difficulty in installing the port from GitHub into a D8 beta6 site, and would love it if we can get a dev branch going here on Drupal.org so I could drushify the entire process :)

Oops, no patch in here yet.

I've started working on a refactoring in the following PR: https://github.com/md-systems/redis/pull/4

It's a mix of 7.x-3.x and the core database backend. Uses the checksum service (including a redis implementation that you can enable), consists of simple hash set/get only for cache items. I'm not sure which approach is faster, but this should work with sharding and I like that it is a lot simpler to understand.

I also started doing some general refactorings to profit from 5.4 language features like traits, like converting the abstract base class to a RedisPrefixTrait.

Could use a review in general and specific about the flush stuff, I'm not 100% sure how much complexity we still need in 8.x (with volatile and so on), the CACHE_TEMPORY stuff is gone.

@Berdir I will be present at DDD Montpellier, and I'll try to sprint in my side for porting Redis to D8 if I manage to have some time to do it. I'll start from what you've done. I'd be happy to discuss this anytime you want there.

@Berdir let me know when you review https://github.com/md-systems/redis/pull/5 I don't have github notifications enabled.

Hello.

I have some time to help on stabilize this module, I cloned from GH and tested a bit with PhpRedis (is working), Predis is throwing some exceptions yet. As there is no issues reported, would be great to have a list of pending tasks to work on.

Thanks in advance.

Testing would definitely be welcome. You might also be able to figure out why the big pending pull request is failing tests but that could be pretty challening.

Only the phpredis (php extension) is currently supported. You're welcome to try and get predis working too.

We now provide redis module with Drupal 8 deployment on Wodby, thanks to @Berdir for D8 version. It seems working well.

I've seen reports of people using the D8 code in production successfully. What's the schedule for bringing it back to Drupal.org and this project? I am working on the Drupal-Redis documentation for Platform.sh, but I am skeptical about recommending it until there's a released module on Drupal.org, even a beta. (Bringing it back to D.o means it's also available via Composer, which is what we're recommending.)

Yes, we've been using the port for more than a year now, on platform.sh, on fairly busy sites. It is working well for us.

The reason it has not yet been moved back to d.o is explained here: #2578085-3: [redis] Redis (comment #3, in case the link does not work).

Problem is, that pull request is not passing the tests, I don't know how to fix it and @pounard didn't have time to look into it. Either someone finds time to fix that branch or @pounard needs to agree to move it back to d.o in the way it is now, which would imply a warning that it can currently not be used with multiple redis shards. Which is not an issue for standard platform.sh environments (nor enterprise, assuming they don't use sharding)

@Berdir I gave you the maintainer access to the Redis project, but please, I'd like you to only manage the 8.x branch, I'll keep the 7.x if it is OK with you. You are free to provide 8.x releases with your actual code.

@pounard: Thank you!

I've now pushed the current code base to drupal.org and created a dev releaes.

If that's OK with you, I will update the project page with a prominent section about the Drupal 8 version and status. In turn, I'd suggest we remove the Drupal 6 part as that version is no longer supported and new users won't need any information related to that I think.

I will definitely not touch the 7.x versions, that's all yours :)

Ok, I've released 8.x-1.0-alpha1 now. Also updated the project page as explained above.

I've added MD Systems as Supporting organization, I suggest you do the same with your company and then remove that part from the Credis section.

Thank you, see you later in the issue queue I guess.

Is there anything specific that currently is a blocker to a beta release? Alpha1 has been out for 5 months now.