ostark/craft-async-queue

Too many jobs => proc_open() fork failed

Closed this issue · 11 comments

khalwat commented

So this isn't really an issue, and I'm sure @ostark you'll say "yeah, that's how it works" but I figured I'd note it for completeness.

The way the Craft job queue works is that it's a synchronous queue. I can push 100 jobs into the queue, and they'll be executed sequentially.

With the async queue plugin, if I push 100 jobs into the queue, each one spawns a new process, and they all execute in parallel.

The side-effect of that is that if you push a whole lot of jobs, or even a few jobs that are very compute-intensive, it can cause an excess of CPU usage/swap.

I realize that much of this is by design; but maybe it makes sense to allow the queue to function synchronously the way the Craft queue runs, but executed via CLI where the restrictions, timeouts, etc. are not a factor as they are and running it via php-fpm?

Here's a real-world scenario that I ran into:

My ImageOptimize plugin adds a job to the queue when a new image is uploaded, to create all of the image transforms necessary. If the user drags in 100 images to upload at once (as they are want to do), this will spawn hundreds of asynchronous queues running simultaneously, doing pretty intensive computations.

The server then is brought to its knees, proc_open() fork failed, etc. There's no limit on the number of these asynchronous queues that can be running in parallel.

phoob commented

I thought this would address that..? 136122f

ostark commented

@khalwat
Thanks for your real world case!
Do you experience the issue with the latest release as well?

136122f should prevent multiple queue runners triggered by the same fpm process, but if multiple fpm processes put a Job into the queue, you end up with multiple runners.

Maybe we can limit this.

ostark commented

I do have a WIP patch to make it more like a synchronous queue. Looks good so far.
@khalwat Can you help with testing your use case?

khalwat commented

I can definitely test it out for you. Maybe something like a "queue pool" where you can set a limit on the number of workers?

ostark commented

I do have a boolean lock so far, but a pool is definitely possible.

khalwat commented

Here's the version I have installed (since it's not versioned via tags, it's coming from dev-master):

        {
            "name": "ostark/craft-async-queue",
            "version": "dev-master",
            "source": {
                "type": "git",
                "url": "https://github.com/ostark/craft-async-queue.git",
                "reference": "c61cac7d6aa85ecfabc5aa434bfac86044cb1e99"
            },
            "dist": {
                "type": "zip",
                "url": "https://api.github.com/repos/ostark/craft-async-queue/zipball/c61cac7d6aa85ecfabc5aa434bfac86044cb1e99",
                "reference": "c61cac7d6aa85ecfabc5aa434bfac86044cb1e99",
                "shasum": ""
            },
            "require": {
                "craftcms/cms": "^3.0.0-RC1"
            },
            "type": "craft-plugin",
            "extra": {
                "name": "AsyncQueue",
                "handle": "async-queue",
                "hasCpSettings": false,
                "hasCpSection": false,
                "changelogUrl": "https://raw.githubusercontent.com/ostark/craft-async-queue/master/CHANGELOG.md"
            },
            "autoload": {
                "psr-4": {
                    "ostark\\AsyncQueue\\": "src/"
                }
            },
            "notification-url": "https://packagist.org/downloads/",
            "license": [
                "MIT"
            ],
            "authors": [
                {
                    "name": "Oliver Stark",
                    "homepage": "https://www.fortrabbit.com"
                }
            ],
            "description": "A queue handler that moves queue execution to a non-blocking background process",
            "keywords": [
                "Craft",
                "cms",
                "craft-plugin",
                "craftcms",
                "queue"
            ],
            "time": "2017-12-05T18:46:11+00:00"
        },
ostark commented

Hey @khalwat & @phoob
I've implemented a process pool to prevent massive concurrency. I would be more than happy if you would give dev-master a try before I tag the 'v1.3' release.

The readme is updated already.

phoob commented

Seems to work well in my dev setup (nginx, php-fpm)

phoob commented

btw this will be a huge improvement, so kudos and 🌮 to you @ostark