jdavidbakr / MultiServerEvent

Laravel plugin to allow scheduled events across multiple servers with the same scheduler to not overlap.
MIT License
37 stars 17 forks source link

using Laravel 5.1 not clearing the lock #9

Closed Arametheus closed 7 years ago

Arametheus commented 7 years ago

So I wrote a simple mailer to just test the scheduler.

So my Kernel file protected $commands = [ Commands\Inspire::class, Commands\SendEmails::class, \jdavidbakr\MultiServerEvent\Commands\MultiServerMigrationService::class, ];

protected function defineConsoleSchedule()
{
    $this->app->instance(
        'Illuminate\Console\Scheduling\Schedule', $schedule = new \jdavidbakr\MultiServerEvent\Scheduling\Schedule
    );

    $this->schedule($schedule);
}

protected function schedule(Schedule $schedule)
{
    $schedule->command('emails:send test@test.com')->everyMinute()->withoutOverlappingMultiServer();
}

id, mutex, lock, start, complete

'52', 'b45c9839f2b6dd7007203ed941dc1d1d', 'wtbeoJnMRIn0c82tB6a5AgJ9CZXxIVFi', '2017-01-29 23:17:02', NULL

it activates this command inserts into the DB as you see above but it never sends the email so I'm guessing some kind of error is happening and never clears it but if I take off the withoutOverlappingMultiServer it sends fine.

any idea's how to debug the issue??

james-coder commented 7 years ago

Do your servers have their time synced all the same within the same second?

Arametheus commented 7 years ago

James, I just have 1 server set up so far. I'm making sure its working first. is there a away for me to run the withoutOverlappingMultiServer with artisan or thru a controller and dd() some stuff I have a feeling its something stupid on my end causing it not to run the command and finish to get to the clear multiserver() function.

jdavidbakr commented 7 years ago

Are you in Laravel 5.4? Version 1.x is not compatible with 5.4. I just released 2.0 which is. Note the change is the Console\Kernel.php file to initialize the scheduler.

Arametheus commented 7 years ago

nope using laravel 5.1 as the title said.. not sure why the issue was closed.

jdavidbakr commented 7 years ago

Try scheduling the inspire command and see if it successfully clears the lock.

Arametheus commented 7 years ago

Same issue I waited for awhile before I took the screen shot below and the time never changed on the start column.

so it looks like I'm getting to line 89 in vendor/jdavidbakr/multi-server-event/src/Scheduling/Event.php

but line 57 never gets called that I can tell so guessing its failing on parent:run($container) or before the run method is called.

if I update the DB and set complete then line 81 does activate and delete the old lock out and gets a new lock.

lock doesn t clear

Arametheus commented 7 years ago

So after I posted the screenshot above. I added debugging emails in the Event.php file just to see what methods are getting hit with calling inspire.

from Kernel I'm calling inspire like this protected function schedule(Schedule $schedule) { $schedule->command('inspire')->everyMinute()->withoutOverlappingMultiServer(); }

first email I get and I get it every minute like I should. from method public function __construct($command) then I get an email from public function skipMultiserver() which guessing thru public function withoutOverlappingMultiServer()

which makes sense since its checking for the lock.

--- Edited so I updated Event.php with one more email to see if it sees if it has the lock or someone else. Its coming back as someone else even when it sets it the first time.

So how is the identity tracked for the server so it knows its the one that set the lock?

-- /Edited

but even on the first call when it sets the lock in the DB it never runs the method public function run(Container $container)

so were in the code does it call this method?

Arametheus commented 7 years ago

So I think what is going on my serverid is changing everytime.

here are a few dumps I emailed my self and I'm only running 1 server so these should be staying the same.

first time my serverid was string(32) "LssqLnYe0xqodILSBf6deHBTF1QBIeHx" second time string(32) "IyjRQFXuM7FVD9USYa2Jin1nPatb3fkP" third time string(32) "6iBupKYBIlmzoA3Ay2UhVTOrLDwH9P7a"

my key is staying the same string(32) "f5128e6f545512502a2ee0e4f8d517db"

so I think it fails at this point in the code if (@$lock[0]->lock == $this->server_id) {

jdavidbakr commented 7 years ago

The 'server_id' value is actually supposed to be unique per run (so that the next run doesn't identify it as its own hash)

What should be happening is the run() command gets called, and then when that commend returns the clearMultiserver() function is called. (Version 2.0 changed this to use the callback, although I don't know whether that was available in Laravel 5.1 or not, but I'm guessing the changes in 2.0 might fix your issue). If you want to give it a whirl, try changing this in the src/Scheduling/Event.php class:

// Remove this function
//    public function run(Container $container)
//    {
//        parent::run($container);
//        $this->clearMultiserver();
//    }
// Change the constructor to this
    public function __construct($command)
    {
        parent::__construct($command);
        $this->server_id = str_random(32);
        $this->then(function() {
          $this->clearMultiserver();
        }
    }

If that solves the issue I can backport that change into the 1.0 branch.

Arametheus commented 7 years ago

Well it didn't error with the new code. just incase you try the code above your missing ); after $this->clearMultiserver(); }

The run() command is still not running. This is what I get when I run php artisan schedule:run with ->withoutOverlappingMultiServer(); No scheduled commands are ready to run.

but if I run it with out ->withoutOverlappingMultiServer(); Running scheduled command: '/usr/local/Cellar/php71/7.1.0_11/bin/php' 'artisan' inspire > '/dev/null' 2>&1 &

jdavidbakr commented 7 years ago

Look at my last comment in https://github.com/jdavidbakr/MultiServerEvent/issues/4 for a possible solution to the run() command. It's got to register the scheduler into the IOC but the instructions only tell you to do it for the Console kernel.

jdavidbakr commented 7 years ago

This code update has been released in version 1.0.7

Arametheus commented 7 years ago

I found another issue for this one.

this part of the code if (@$lock[0]->lock == $this->server_id) { // We got the lock return false; } the lock is an array not an object so this part is failing $lock[0]->lock

if I change it to this if (@$lock[0]['lock'] == $this->server_id) { // We got the lock return false; }

its working perfectly and with the new code you gave me above.

jdavidbakr commented 7 years ago

That's been changed in version 2 as well, I just ported the change into 1.0.8

Arametheus commented 7 years ago

Woot perfect.. Thank you.