Not compatible with php-redis 5

Hi, we're also seeing the same error popup on our sites and it's causing that some content doesn't show the changes made until much later. I'll keep an eye on this issue.

I'd guess this isn't caused by the used redis version but the redis client, maybe platform.sh updated the version of the extension that's used? What PHP version are you using?

Obviously nothing in 8.7.5 could influence this and there were also no changes in this module.

Looks like there recently was a new redis extension release, 5, with deprecated changes: https://pecl.php.net/package-changelog.php?package=redis&release=5.0.0.

See https://platformsh.slack.com/archives/C0JHEUHQD/p1563482755352700, as I suspected, this is caused by the update to php-redis 5, apparently only in the PHP 7.3 images.

So, workarounds for now: either switch to the php library instead of the extension or downgrade to PHP 7.2

That's just a workaround, lets keep this open.

We'll need to at least communicate about this, or implement it so that it works with php-redis 5.0 too. @DamZ also said in slack that he recommends to use the php library instead, even though it is marginally slower.

I'm getting a full log of Redis errors:

Deprecated function: Function Redis::delete() is deprecated in Redis_Lock_PhpRedis->lockRelease() (line 111 of /web/sites/all/modules/contrib/redis/lib/Redis/Lock/PhpRedis.php).

Drupal 7.67
Redis 7.x-3.17
PHP 7.1.31
php-redis 5.0.2+4.3.0-2+ubuntu18.04.1+deb.sury.org+1

That started after I upgraded the server (apt upgrade).

> php-redis 5.0.2+4.3.0-2+ubuntu18.04.1+deb.sury.org+1

Yes, this isn't supported at the moment, you need to downgrade or switch to the php library.

Thanks for a quick answer @Berdir.

Meanwhile I wanted to track down this issue as it appears from the error message it's just the delete function that's deprecated.
It should be calling del() instead by looking at Redis 5 documentation.

There are just two references in Redis client object in Drupal 7.x-3.x-dev version where delete() is used instead of del() which I changed and made a patch.

Works fine for me now. No deprecated errors any more.

Ah, I missed that you are on D7. Can you create a separate issue for that maybe? redis 8.x and 7.x have two separate maintainers, so it's easier to keep the issues separate, the code base is also completely different anyway.

Because the D8 version has bigger issues and doesn't seem to work at all anymore.

Hmm i have php 7.2.20 and redis 4.0.9 and also getting this nasty "Redis::multi(): Can't activate pipeline in multi mode! "

guessing it could be then in later 7.2.x versions also.

The php version doesn't matter, the version of php-redis does, which is a standalone extension.

PHP 7.2 vs 7.3 only matters on platform.sh where the extension is part of the whole bundle

Redis-cli version stated 4.x version but when I got to phpinfo I see I have also 5.0.2 version installed. Thanx on info. must be that then.

D7 which has the same problem with php-redis 5 - moved to an own issue: https://www.drupal.org/project/redis/issues/3074189

Thanks, back to active as this doesn't have a patch yet.

I checked the D8 version for same error and only found one reference where redis client is called with delete() method. Other are using del().
Fix in patch is solving the second error in the first comment.

The first one Redis::multi(): Can't activate pipeline in multi mode might be solved if Redis::MULTI (default) mode is used instead of Redis::PIPELINE.

Giving it another go to solve both errors from the initial post.
Default Redis::MULTI mode used instead of Redis::PIPELINE.

@alesr there is a second case where Redis::PIPELINE is being used, I've updated the patch accordingly.

It seems there is a minor glitch in the patch:

All src/Cache/PhpRedis.php of our projects have

    $pipe = $this->client->multi(\REdis::PIPELINE);

instead of

    $pipe = $this->client->multi(\Redis::PIPELINE);

That typo was fixed in the dev version, the patch needs to apply against that.

I'm not sure if multi() without that argument is still the same/what we need. This will need to be extensively tested, with both php-reds 5 and older versions.

As a start, it would be helpful if someone could set up a pull request against https://github.com/md-systems/redis, bonus points for setting up travis to test with both extension versions, if that is possible...

@melsi the patch applies against the dev release of this module, like @Berdir already mentioned.

Hello,

Thanks for the patch from comment 21.

Patch #21 worked for me.

Thank you. Patch #21 solves the problem.

I have updated the tests on travis and can confirm that php 7.3 there now also fails with the delete error:

https://travis-ci.org/md-systems/redis/jobs/582905983

I haven't been able to reproduce the multi() related error though, doesn't seem to happen for the tests. Are you maybe using this in combination with certain configuration like persistent connections?

See https://github.com/md-systems/redis/commits/phpredis5, you can see that both the commit with that change and that without passed tests.

I plan to commit this ASAP, but would like to understand it a bit better first.

Decided to go ahead with the patch. I don't know if there's a performance difference between running the comments as pipeline or multiple and maybe multi even makes more sense, to avoid having multiple identical sets overlap.

If this helps in some cases, fine. Tests can't really simulate more complex cases/heavy load.

Thanks everyone for providing patches and testing.

Thanks @Berdir.
Nice to see that one fixed now!
You might have the wrong author of the commit though but not sure what the rules are with that.

Yes, that's always tricky with multiple authors. Everyone has issue credits though, the commit author isn't that important.

We had a related issue.

Our site was running on PHP 7.1.32, Drupal 8.7.6, Redis Module 1.1, Redis Client 3.1.4RC1 pretty robust for months.

Then platform.sh did a security patch wich accidentally upgraded the Redis Client Version to 5.0.2.

Our error.log was full of this error:
Fatal error: Redis::multi(): Can't activate pipeline in multi mode! in /app/web/modules/contrib/redis/src/Cache/PhpRedis.php on line 96

We then implemented the patch #21 for the Redis Module (1.2 was not released at that time) and it seemed that the website was working again fine.

Then platform.sh reverted the Redis Client Version back to 3.1.4RC1 and we removed the patch #21 again. With that composer update we also upgraded to Redis Module 1.2.

After this a few deployments to the production environment had a strange behavior. Generated JS-Files and/or CSS-Files were not generated properly or were missing. We did not dive deep in what exactly happened and reverted the Redis Module back to 1.1.

Now the site seems to run fine again.

Could it be that Redis without Pipeline in Multi mode can cause such errors with the static JS- and CSS-Files or other issues?

> Could it be that Redis without Pipeline in Multi mode can cause such errors with the static JS- and CSS-Files or other issues?

Maybe? If you manage to reproduce it somehow or if it happens more frequently then please open a new issue. I'd also be interested in information on how/when these fatal errors happen, per my comments above I couldn't reproduce them in the tests.

I'd also be interested in information on how/when these fatal errors happen, per my comments above I couldn't reproduce them in the tests.

The "Can't activate pipeline in multi mode!" happens when the set method (redis/src/Cache/PhpRedis.php on line 96) calls:

$pipe = $this->client->multi(\REdis::PIPELINE);

It seems that in newer Versions this "experimental feature" is no longer available (Pipeline usage in "multi" mode).

The latest version of this module now removes the "Redis::PIPELINE" parameter in the clients "multi" method for all Redis versions, which could lead to a slower/different behavior than before.

> It seems that in newer Versions this "experimental feature" is no longer available (Pipeline usage in "multi" mode).

newer versions of what? As I said, the tests worked just fine with php 5.3 + phpredis5 on TravisCI, with that change still there, it's also still documented as an available argument, only in certain conditions int fails and I don't know when.

Line 2336 of https://github.com/phpredis/phpredis/blob/7ef17ce1a3e6d417138b6f51bb1241...

if (multi_value == PIPELINE) {
        /* Cannot enter pipeline mode in a MULTI block */
        if (IS_MULTI(redis_sock)) {
            php_error_docref(NULL, E_ERROR, "Can't activate pipeline in multi mode!");
            RETURN_FALSE;
        }

        /* Enable PIPELINE if we're not already in one */
        if (IS_ATOMIC(redis_sock)) {
            free_reply_callbacks(redis_sock);
            REDIS_ENABLE_MODE(redis_sock, PIPELINE);
        }
    } else if (multi_value == MULTI) {
[...]
}

The error seems to be related to the mode the Redis Server is in.

"IS_MULTI" is defined in https://github.com/phpredis/phpredis/blob/7ef17ce1a3e6d417138b6f51bb1241... on line 116:

#define IS_MULTI(redis_sock) (redis_sock->mode & MULTI)