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

Use of Redis yields "job priority is not supported in the driver" error #201

Closed
martinhellwagner opened this issue Apr 2, 2020 · 31 comments
Labels
bug Something isn't working

Comments

@martinhellwagner
Copy link

Describe the bug

We're using Redis for our Craft CMS queue. On top of it, we're using Blitz to cache certain sites that receive lots of traffic. However, using Blitz with Redis always results in the error message "job priority is not supported in the driver" when trying to upload an image or create an entry. It doesn't matter whether or not we're using Blitz File Storage or Yii Cache Storage, and whether or not we're moving cache to Redis – always the same error.

To reproduce

Steps to reproduce the behaviour:

  1. Install Redis and Blitz on Craft CMS.
  2. Try to upload an asset or create an entry.

Expected behaviour

Upload / creation should be fine.

Screenshots

Screenshot 2020-04-02 at 17 17 51

Versions

  • Plugin version: 3.6.2
  • Craft version: 3.4.10.1
@martinhellwagner martinhellwagner added the bug Something isn't working label Apr 2, 2020
@martinhellwagner
Copy link
Author

martinhellwagner commented Apr 2, 2020

Seems like when I remove the call where the priority of the Craft queue is set in the source code, then it's working fine. Would it be an idea to set the priority to -1 or something like that when you don't want to (or can't) have priority on your queue, and check it in the source code like this?

if ($priority >= 0) {
  $queue->priority($priority);
}

@bencroker
Copy link
Collaborator

Hmm, are you using yii\queue\redis\Queue? It extends yii\queue\Queue which contains the priority method, so should still work.

@martinhellwagner
Copy link
Author

martinhellwagner commented Apr 2, 2020

Yes, this is what our config/app.php looks like:

    'components' => [
        // Initialize Redis component
        'redis' => [
            'class' => yii\redis\Connection::class,
            'hostname' => 'localhost',
            'port' => 6379,
            'password' => null,
        ],
        // Use queue with Redis
        'queue' => [
            'class' => yii\queue\redis\Queue::class,
            'redis' => 'redis',
            'channel' => 'queue',
        ],
    ],

@martinhellwagner
Copy link
Author

Apart from that, we're requiring the Yii Redis Component in the composer.json like this:

"yiisoft/yii2-redis": "^2.0"

Our servers are provisioned by Forge, so Redis is baked in there. Is there anything we are doing wrong?

@martinhellwagner
Copy link
Author

I stumbled upon this issue, where it's claimed that Redis hasn't implemented job priority: yiisoft/yii2-queue#205

@bencroker
Copy link
Collaborator

Thanks, will look into adding a workaround soon.

@martinhellwagner
Copy link
Author

martinhellwagner commented Apr 2, 2020

Thank you! 🙂

Since we need the functionality pretty soon, do you see any problems in forking the plugin for now and commenting out the two lines in the source code where priority is set?

@bencroker
Copy link
Collaborator

Fixed in 6f6c115 on the develop branch, so you can use that until the next release.

@martinhellwagner
Copy link
Author

Wonderful, cheers! 😍

@martinhellwagner
Copy link
Author

@bencroker

Just installed the fix and tested it: unfortunately, this doesn't solve the problem, as Craft queue indeed has a priority – just the Redis queue doesn't.

Is it possible to check in the if whether or not Redis is used as a queue driver – or maybe actually use a parameter in the config file to do so?

@bencroker
Copy link
Collaborator

Yeah, you're right, the Redis queue just throws an error if a priority is set.
https://github.com/yiisoft/yii2-queue/blob/2.3.0/src/drivers/redis/Queue.php#L190-L192

Will work on another fix for this.

@bencroker
Copy link
Collaborator

Haven't had a chance to test, but the priority is now not set when using a Redis queue in 042a9f3.

@martinhellwagner
Copy link
Author

Will test right away!

@martinhellwagner
Copy link
Author

Yes, that looks very good! Thanks again for the quick fixes! 💯

@martinhellwagner
Copy link
Author

martinhellwagner commented Apr 3, 2020

One last question: when we deploy a new release, we naturally want the Blitz caches to be in that release as well. That's why we have the CLI command ./craft blitz/cache/refresh in our pipeline. But when the command is called in the pipeline (or directly in the Craft CLI), there's no cache/blitz folder to be found in the new release.

I'm getting the situation of Screenshot 1 (left: new release, right: old release) whereas I'd be expecting the scenario of Screenshot 2 after the CLI command – which I'm only getting after I've visited the respective cached page.

Screenshot 2020-04-03 at 12 01 54

Screenshot 2020-04-03 at 12 03 47

@martinhellwagner
Copy link
Author

Seems like ./craft blitz/cache/warm works fine in the pipeline and in the CLI. 🤔

@martinhellwagner
Copy link
Author

By the way, I think also the Craft guys fixed the issue on their side: craftcms/cms#5876

@bencroker
Copy link
Collaborator

They did, and I've since followed suit with catching an exception in 167f277.

@martinhellwagner
Copy link
Author

martinhellwagner commented Apr 4, 2020

@bencroker

Just realized that not only the ./craft blitz/cache/refresh CLI command works differently than expected (see above), but also the ./craft blitz/cache/refresh-expired seems to be crashing when the queue is in Redis:

Screenshot 2020-04-04 at 21 01 39

@martinhellwagner
Copy link
Author

martinhellwagner commented Apr 4, 2020

Also, it seems our cached HTML files get deleted, even though we don't have a specific cache duration set (from the config file, I see that the default value 0 means unlimited caching). Is it a good idea to activate the "Warm Cache Automatically" parameter in the settings?

@bencroker
Copy link
Collaborator

bencroker commented Apr 4, 2020

Ok, hopefully dee62a9 will fix the error in the console.

Just realized that not only the ./craft blitz/cache/refresh CLI command works differently than expected

The refresh command will clear, flush and purge the cache, and then warm and deploy it only if the Warm Cache Automatically setting is enabled.

@martinhellwagner
Copy link
Author

Looks good, thanks! 😍

@martinhellwagner
Copy link
Author

@bencroker

Is there any setting that clears the cached HTML files when the expiry date is not specifically set (and thus infinite) and the "Clear Cache Automatically" parameter disabled? Our caches are rebuilt periodically whereas they should always have the same cached file.

Maybe Craft caching is still interfering here?

@martinhellwagner
Copy link
Author

This could also be related to #70?

@bencroker
Copy link
Collaborator

Have you disabled Craft caching by setting enableTemplateCaching to false in the general config?

What action triggers a cache refresh? No files should be deleted if Clear Cache Automatically is disabled, until refresh-expired is called. If this persists then please open a new isssue with some details and I'll help troubleshoot there.

@martinhellwagner
Copy link
Author

No, enableTemplateCaching is set to true in our Stage and Production environments. Reason for this is that we're actually not caching all the pages with Blitz (just the most visited ones) and rely on Craft caching for the others. Creating and saving an entry both triggers the Blitz Cache rebuild.

Guess the "dual" caching strategy is the problem?

@bencroker
Copy link
Collaborator

While using both caching systems together can cause issues, it shouldn't lead to pages cached by Blitz being cleared unexpectedly.

@martinhellwagner
Copy link
Author

Just tried setting the parameter to false and creating an entry again – still happens. I'll research some more, but I might have to open an issue about this. 🤔

@john-henry
Copy link

Im getting this now when I upload an Asset. I have narrowed it down to Blitz

Craft 3.7.39
Blitz 3.12.2

I only installed Redis this morning and all of that works fine

Here is Sentry error https://sentry.io/share/issue/e483de1948d04ac4bdf28030f73f9990/

I do use enableTemplateCaching set to true but I have tested on or off and same issue uploading

@john-henry
Copy link

john-henry commented May 2, 2022

Now Im not even sure if any Blitz tasks have made the new queue today. Am I missing some config option?

@bencroker This is on saving entries too. You want me to make a new issue or its ok to live here?

@bencroker
Copy link
Collaborator

This issue is 2 years old, new issue please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants