Skip to content

The Redis INFO command called in RedisTrait::doClear can fail: "The command 'INFO' is not allowed in replication mode." #35867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
msarris opened this issue Feb 26, 2020 · 12 comments
Closed

The Redis INFO command called in RedisTrait::doClear can fail: "The command 'INFO' is not allowed in replication mode." #35867

msarris opened this issue Feb 26, 2020 · 12 comments
Labels
Bug Cache Status: Needs Review

Comments

@msarris
Copy link

msarris commented Feb 26, 2020 •
edited
Loading

Symfony version(s) affected: 4.4, 5.0

Description
In \Symfony\Component\Cache\Traits\RedisTrait::doClear there's a method call $host->info('Server') which executes the Redis INFO command.

Currently there's no exception handling on this call, but I think there should as this command can throw an exception, at least the following:

The command 'INFO' is not allowed in replication mode.

How to reproduce
I'm not sure how to easily reproduce this issue. I see the logs in production, but only occasionally, so it only occurs when the Redis instance(s) are in replication mode, which is not all the time, since the error doesn't usually happen.

Possible Solution
Add a try catch statement to defend against the possible failing of this call and to not interfere with clearing the cache.

Additional context
The full log message I currenly get is the following (I use the Predis client):

The command 'INFO' is not allowed in replication mode. {"exception":"[object] (Predis\NotSupportedException(code: 0): The command 'INFO' is not allowed in replication mode.
@msarris msarris added the Bug label Feb 26, 2020
@msarris msarris changed the title The Redis INFO command called in RedisTrait::doClear can fail: "The command 'INFO' is not allowed in replication mode. The Redis INFO command called in RedisTrait::doClear can fail: "The command 'INFO' is not allowed in replication mode." Feb 26, 2020
@scaytrase
Copy link
Contributor

Yea, for me it happened like this

bin/console cache:pool:clear cache.app.metadata_provider -vvv

 // Clearing cache pool: cache.app.metadata_provider

2020-03-02T17:39:40+03:00 [warning] Failed to clear the cache: The command 'INFO' is not allowed in replication mode.
 [OK] Cache was successfully cleared.

Very interesting to have failed to clear the cache and Cache was successfully cleared in single output (warning even is not visible without -vvv)

@DemigodCode
Copy link
Contributor

Predis is not maintained anymore, so I forked the redis client.
https://github.com/antibodies-online/predis

BUT that needs a little bit attention to the configuration of redis. Best all versions of your redis instances are equal!

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@nicolas-grekas
Copy link
Member

PR welcome!

@carsonbot carsonbot removed the Stalled label Feb 19, 2021
@DemigodCode
Copy link
Contributor

No still using my fork currently.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 19, 2021 •
edited
Loading

@DemigodCode if you think we should patch Symfony, can you please send us a PR?
If predis should be patched, can you open a PR on https://github.com/predis/predis please? (and link it here)

@DemigodCode
Copy link
Contributor

DemigodCode commented Feb 19, 2021 •
edited
Loading

@nicolas-grekas don't think my changes will work for everyone.
I've just allowed the info command in replication mode. That implies to have the redis same version on each server.
At the moment I have no idea to fix it in a general way.
I don't clearly remember, but there is a switch somewhere which tries do determine the server version in the caching adapter.
Maybe the support of older redis versions should be dropped there in future symfony versions, that will obsolete the info request.

@nicolas-grekas
Copy link
Member

Maybe removing that line would work?
https://github.com/predis/predis/blob/338ba6d73d1293259953793a2c2b41e509b964c1/src/Replication/ReplicationStrategy.php#L194
Can you give it a try and submit a PR on predis if it works?
Then we might find a way around in Symfony.
Let me know.

@DemigodCode
Copy link
Contributor

I'll give it a try next week and will let you know.

@DemigodCode
Copy link
Contributor

DemigodCode commented Feb 19, 2021 •
edited
Loading

predis/predis#598

Long long time ago... Just remembered... They didn't want to "activate" the info command in replication mode.
Guess best will be to check for master as nrk states there.

symfony/src/Symfony/Component/Cache/Traits/RedisTrait.php

Line 373 in dc2635c

$info = $host->info('Server');

symfony/src/Symfony/Component/Cache/Adapter/RedisTagAwareAdapter.php

Line 286 in dc2635c

$info = $host->info('Memory');

@nicolas-grekas
Copy link
Member

Give that both calls happen on a path when a write is going to happen, going to the master is not a problem to me.
Can you send a PR to do so?

@DemigodCode
Copy link
Contributor

Yes, next week. Have to check if phpredis has the same interface.

This was referenced Feb 25, 2021
Closed
Merged
nicolas-grekas added a commit that referenced this issue Feb 25, 2021
@nicolas-grekas
bug #40299 [Cache] Add server-commands support for Predis Replication…
3fe1564 
… Environments (DemigodCode)

This PR was merged into the 4.4 branch.

Discussion
----------

[Cache] Add server-commands support for Predis Replication Environments

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #35867
| License       | MIT
| Doc PR        |

This fix is for predis MasterSlaveConnections which don't allow to run server commands.
Due to that it's not possible to e.g. clear a cache with cache:pool:clear.

PhpRedis and Predis do not have the same interface, so have to check which implementation is used.
Furthermore, the getClientFor('master') works only for replicated redis instances.

Commits
-------

2ae5c33 [Cache] Add server-commands support for Predis Replication Environments
@acelaya acelaya mentioned this issue Feb 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Cache Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@javiereguiluz @nicolas-grekas @msarris @scaytrase @carsonbot @DemigodCode