laravel / lumen-framework

The Laravel Lumen Framework.
https://lumen.laravel.com
MIT License
1.47k stars 419 forks source link

queue:work creates multiple defunct processes and eats up too much memory #510

Closed djoks closed 7 years ago

djoks commented 8 years ago

I am using lumen 5.3 and followed the docs on how to schedule a job. I did everything as instructed and it worked but then I noticed that my server kept running out of memory and crashing I investigated the issue and found out that mysql used up too much memory and crashed. I traced it to the job I was running since I was using the database for my queues.

So then I switched to redis which eliminated the issue of mysql eating up my memory but then my server still crashed and I had to reboot.

Support says it was because the queue:work command had over 40 processes in memory and a lot of them were defunct so I checked my processes list and realized this was true. Somehow for one simple job I was running the queue:work command creates many and many processes till eventually my server runs out of memory and crashes.

Dispatching job

$this->dispatch(new SendDepositSMSAlertJob($bank . ' - ' . $acNumber, $depositType, $depositAmount, $reportDate, $telNumbers));

SendDepositSMSAlertJOB

<?php

namespace App\Jobs;

class SendDepositSMSAlertJob extends Job
{
    private $number;
    private $bank;
    private $accountNo;
    private $type;
    private $amount;
    private $date;

    /**
     * Create a new job instance.
     *
     * @return void
     */
    public function __construct($bank, $accountNo, $type, $amount, $date, $number)
    {
        $this->number = $number;
        $this->bank = $bank;
        $this->accountNo = $accountNo;
        $this->type = $type;
        $this->amount = $amount;
        $this->date = $date;
    }

    /**
     * Execute the job.
     *
     * @return void
     */
    public function handle()
    {
       // Settings
        $url = "https://api.xxxxxx.com/v3/messages/send";
        $from = "XX";
        $message = urlencode("Your account at " . ucfirst(strtolower($this->bank)) . " ending with " . substr($this->accountNo, count($this->accountNo) - 5, 4) ." has been credited with a " . strtolower($this->type) . " deposit of XX " . number_format($this->amount, 2, ".",",") . " on " . $this->date . ".");

        $client_id = "xxxxxx";
        $client_secret = "xxxxxx";
        $query_string = "?From=".$from."&To=".$this->number."&Content=".$message."&ClientId=".$client_id."&ClientSecret=".$client_secret."&RegisteredDelivery=true";

    $ch = curl_init();
        curl_setopt($ch, CURLOPT_URL, $url.$query_string);
        curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
        $response = curl_exec ($ch);
        curl_close ($ch);
    }
}

Now I used this same SMS script/job in another laravel app and have been doing so for over a year now and never had an issue like this, that app was v5.2 so now I am totally confused here. How on earth can this be a code issue? If it is then it's certainly not MY code.

Screenshots

http://prnt.sc/cx0e2s http://prnt.sc/cx0e8b http://prnt.sc/cx0efp

bobbybouwmann commented 8 years ago

This sounds like a code issue and not a Laravel issue. I have an API running on Lumen 5.3 that has over 100.000 requests per hour and runs over 5000 jobs per hour without any issues.

If you want us to inspect your code on issues I would kindly ask you to try it on the Laracasts forum ;)

djoks commented 8 years ago

Well I can certainly post the code here for you to see if you're that certain it's a code issue.

djoks commented 7 years ago

This is not a code issue, all my code does is send an SMS via cURL. schedule:run just hangs and so does queue:work even when there are no schedules to run it just hangs and piles up till my server crashes.

laurencei commented 7 years ago

@djoks - it is definitely a code issue.

I run large lumen queues without issue, as do many others.

If you have a memory leak somewhere - then just run queue:listen instead - which will cycle your entire framework call and should remove memory issues.

all my code does is send an SMS via cURL

I note that you are using file_get_contents() - that is not cURL. You should really use Guzzle or similar.

I also note you are suppressing the errors for @file_get_contents() - so you could easily be hiding errors which is causing your application to crash.

djoks commented 7 years ago

Actually this code is old, I switched to cURL afer deciding that it might be an issue with file_get_contents but I still have the same problem. I even took out the loop so that instead of one job executing 4 times I had 4 separate jobs each executing once. Will update my original post to show my current code. If you can see something wrong with it then let let me know coz I honestly cannot.

laurencei commented 7 years ago

Did you try switching to queue:listen? That will reboot the whole framework on each call, ensuring all memory is released.

djoks commented 7 years ago

When I use queue:listen or queue:work I don't seem to have any issues (at least not for the short duration I tested it for), however when I use schedule:run to run queue:work from cron that's when everything starts going wrong.

laurencei commented 7 years ago

oooooooooo - THAT is your problem.

You should never be running queue:work from the scheduler!!

You should be setting queue:work as its own daemon process which is constantly running. And have the scheduled jobs push onto the queue separately.

I suggest asking about this on the forums to discuss further.

djoks commented 7 years ago

Ohh alright, could have sworn I followed the docs though, anyway so then do you suggest I create a second cron to handle queue:work?

djoks commented 7 years ago

Just to be clear about what you are saying this is my kernel.php

<?php

namespace App\Console;

use Illuminate\Console\Scheduling\Schedule;
use Laravel\Lumen\Console\Kernel as ConsoleKernel;

class Kernel extends ConsoleKernel
{
    /**
     * The Artisan commands provided by your application.
     *
     * @var array
     */
    protected $commands = [
        //\JK\Dingo\Api\Console\Commands\RouteListCommand::class
    ];

    /**
     * Define the application's command schedule.
     *
     * @param  \Illuminate\Console\Scheduling\Schedule  $schedule
     * @return void
     */
    protected function schedule(Schedule $schedule)
    {
        // Run once a minute
        $schedule->command('queue:work')->everyMinute()->sendOutputTo(storage_path('logs/scheduler.log'), true);
    }
}
laurencei commented 7 years ago

so then do you suggest I create a second cron to handle queue:work?

No - you dont use cron to do queue:work at all. You need to create a daemon process that has nothing to do with cron.

You'll have to ask on the forums - this isnt the right place to discuss.

Edit: this is a starting point: https://laracasts.com/discuss/channels/forge/create-a-single-queue-worker-processing-multiple-queues