Skip to content
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.

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

cache:prune-stale-tags gives call to undefined method RedisCluster::pipeline() #53927

Closed
joostdebruijn opened this issue Dec 16, 2024 · 6 comments

Comments

@joostdebruijn
Copy link
Contributor

Laravel Version

11.35.1

PHP Version

8.4.1

Database Driver & Version

No response

Description

After upgrading to v11.35.1 the hourly scheduled command cache:prune-stale-tags results in a Call to undefined method RedisCluster::pipeline() when running the application with the native php-redis extension at, in this case, Laravel Vapor.

The issue can be 'resolved' by downgrading the application to v11.35.0 or installing Predis. I think the issue is related to PR #53837 (and issue #53826), but I don't see directly what is causing the call tot pipeline().

Steps To Reproduce

  1. Upgrade the application to Laravel 11.35.1 without installing Predis
  2. Connect the application to a Redis Cluster-instance
  3. Run cache:prune-stale-tags
  4. Result: Call to undefined method RedisCluster::pipeline()
@crynobone
Copy link
Member

AFAIK cache:prune-stale-tags was never working on Redis Cluster (via phpredis driver) since pipeline() is not available from PhpRedisCluster implementation:

@bentleyo
Copy link
Contributor

@joostdebruijn for us the cache:prune-stale-tags command would always timeout before scan was fixed in #53837. This happened because the flushStaleTags method would scan redis for tags, but the wrong parameters were sent when redis was in cluster mode and a count of only 10 was being fetched per scan instead of the 1000 it intended.

This timeout meant for us that the script was never able to fetch all the keys so it never got to the point of calling flushStaleEntries which is where that pipeline call happens.

It also caused large spikes in command calls every hour (which is when we ran it) for a couple minutes until it timed out.
Screenshot 2024-12-17 at 5 50 26 pm

I think the issue has always been there as the RedisCluster class doesn't support pipeline as @crynobone mentioned. The timeout was just happening before this exception could happen.

I've been running into quite a few issues when using redis in laravel with clusters. Some of those are because of bugs in phpredis itself: phpredis/phpredis#2601 and phpredis/phpredis#876 (relates to #53266). For us we're using redis serverless setup by vapor which is always in cluster mode.

@joostdebruijn
Copy link
Contributor Author

Thanks for your reply, @bentleyo! I never noticed this, but I see a similar pattern in our environment too (we're also using Redis serverless by Vapor). Nice that it has been fixed! As I understand it correctly, you are using predis to circumvent the limitations of phpredis?

@bentleyo
Copy link
Contributor

@joostdebruijn no problem! The issue I resolved wasn't specifically to do with tags in Laravel, but scanning (a feature that tags depends on).

We noticed our Redis serverless DB was getting huge (10+GB) as the tag metadata wasn't being cleaned up. Again, because cache:prune-stale-tags was timing out and never actually pruned as a result. We started periodically clearing cache as a temporary workaround to prevent a large bill.

To be honest I haven't really looked into the issue with pipeline you reported. There might be a path forward there with Predis, but I'm mainly commenting to point out the exception isn't a result of my PR and provide some context 😇

For us we've moved away from using tagged cache as it's no longer a feature that Laravel wants to encourage the use of (that's what Taylor said in a Q&A at Laracon AU) and as such the documentation for it has been removed from the Laravel documentation.

@bentleyo
Copy link
Contributor

@joostdebruijn the Predis wiki says it supports:

Command pipelining (works on both single and aggregate connections).

So that might be worth looking into. However, we ran into issues using Vapor setup serverless Redis with Predis (a healthcheck fails on deployment). I think that's because Vapor overrides the redis config and is missing some config for Predis to work (scheme and ssl.cafile) per cluster config.

There's also another issue where PredisClusterConnection in Laravel doesn't support scanning propery. Similar to my PR, but for Predis.

As you can tell, I'm having a fun time with this too 🙃

@joostdebruijn
Copy link
Contributor Author

@bentleyo Thanks for all the valuable insights! Will close this issue for now, I think the best way forward for the long term is to move away from cache tags then. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants