search

putyourlightson / craft-blitz

Intelligent static page caching for creating lightning-fast sites with Craft CMS.
https://putyourlightson.com/plugins/blitz
Other
149 stars 36 forks source link

Automatic cache warming / remote deployment doesn't work with Async Queue plugin #384

Closed mgburns closed 2 years ago

mgburns commented 2 years ago

Describe the bug

We're setting up remote deployment to Netlify following your guide. It works fine with the default queue system, but when we introduce out-of-band queue runners via the Async Queue plugin the deployments are corrupt. For example -- saving an edit to the homepage will consistently remove all index.html pages from the deployed site.

To reproduce

Steps to reproduce the behaviour:

  1. Configure remote deployment to Netlify via the Git Provider as described in this guide
  2. Install and enable the Async Queue plugin
  3. Save an edit to any page on the site

Expected behaviour

Update page is deployed to Netlify

Actual behaviour

All pages are removed in the commit that is deployed to Netlify

Additional context

I thought it might be due to having multiple Async Queue runners, but even with a single queue runner (setting ASYNC_QUEUE_CONCURRENCY=1 in your .env file) this behavior persists.

It seems clear that it's a timing issue -- the cache is cleared and deployed before the warming is complete. I have a hunch that it has to do with the fact that the Async Queue runner is executed via the console and not a web requests, and that this plugin is tripping up on that.

Screenshots

image

Versions

bencroker commented 2 years ago

The local warmer (experimental) is known to have issues when run via console commands, but we're reworking it for Blitz version 4. Are you using it in this case?

mgburns commented 2 years ago

@bencroker We're actually using the Guzzle warmer.

bencroker commented 2 years ago

My guess is that the warming process is failing. Have you tested it without the Async Queue plugin?

mgburns commented 2 years ago

@bencroker We have -- switching to the built-in queue runner fixes this issue, but isn't ideal.

We've encountered a few other issues recently that appear to involve similar queue-induced race conditions. For example:

./craft blitz/cache/refresh --queue=1
Blitz cache successfully cleared.
Blitz cache successfully flushed.
Cache purging is disabled.
Warming Blitz cache...
Blitz cache queued for warming.
Deploying pages...
[==============================================================================] 100% (197/197) ETA: n/a
Deploying complete.

Passing --queue=1 is supposed to trigger queue jobs instead of running them directly, but as you can see from this log it's inconsistent -- the warming is queued, but the deployment runs immediately.

Our solve there is just to drop the --queue flag, but this behavior is inconsistent with other Blitz Console commands. For example, running ./craft blitz/cache/refresh-tagged foobar was removing all pages from the remote deployment because the warming happened via a queue job and the deployment happened immediately. In that case, adding --queue=1 fixed it by causing both warming and deployment to happen in queue job.

Happy to file this as a separate issue, but I have a hunch that the underlying cause is related. It seems like having separate jobs for warming and deploying is problematic as there is no guarantee that they will run sequentially. This leads to some pretty scary possibilities (ex: inadvertently deleting every page during remote deployment).

FWIW we're paying for the plugin, happy to share more details in a DM if it'd be helpful.

bencroker commented 2 years ago

Ok, I'd be happy to take a closer look and fix any inconsistencies. A new issue with as much detail as possible would be appreciated.

mgburns commented 2 years ago

@bencroker Happy to open another issue, but I don't think this one has been resolved. What was your logic for closing it?

bencroker commented 2 years ago

From your previous comment, it appears that the Async Queue plugin is causing that particular issue. It's not the first time I've seen this plugin cause problems, so I suggest you either use the built-in queue runner or, if you prefer something more robust, then one of the other solutions described in https://nystudio107.com/blog/robust-queue-job-handling-in-craft-cms

mgburns commented 2 years ago

Got it. I'll give those a shot, but pretty sure it's the same underlying architecture as Async Queue and is subject to the same issue -- the Rewarm and Deploy jobs this plugin queues are not run deterministically when there are multiple queue listeners, which can lead to broken deployments. I'll let you know if that's the case -- if so it might be helpful to note this incompatibility in the docs.

bencroker commented 2 years ago

I'll take care of the inconsistencies in your new issue, once created.

bencroker commented 2 years ago

The inconsistencies should be fixed in https://github.com/putyourlightson/craft-blitz/commit/8d5a8778e02720e74479e9c8a937f813bacd0695 for the next release.

mgburns commented 2 years ago

Thanks, @bencroker!

mgburns commented 2 years ago

@bencroker Just wanted to confirm that 3.12.0 did indeed fix the inconsistencies I reported above.

Also worth mentioning that with this change we were able to re-enable the Async Queue plugin so long as there was only 1 queue runner (e.g. setting ASYNC_QUEUE_CONCURRENCY=1 in our .env file). Any more than 1 queue handler and you hit the original issue -- broken deploys due to concurrent processing of the separate cache warming and deploy queue jobs.

bencroker commented 2 years ago

Ok, that does indeed make sense!