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

Don't use Carbon::now(), use DB's time #10

Closed james-coder closed 7 years ago

james-coder commented 7 years ago

Instead of using Carbon::now() everywhere, this should be using the DB's now(). That way, even if your servers aren't synced up exactly to the same second, things will not break.

This will require using the DB's NOW() on the insert and on the where clauses.

https://github.com/jdavidbakr/MultiServerEvent/blob/master/src/Scheduling/Event.php#L84

(Even if the DB has the wrong time and you have multiple servers using this library, it will still work because they will all be consistently wrong instead of inconsistent.)

jdavidbakr commented 7 years ago

I am going to refrain from using DB::Raw() calls

james-coder commented 7 years ago

Servers should be synchronized within a second of each other in order to prevent this error though, right? (Maybe just mention that in the documentation if you'd hate to use DB::raw()?)

boynet commented 7 years ago

Hi any good reason? DB:raw is not evil 👍 and he has a good point about servers time synchronized

jdavidbakr commented 7 years ago

DB:raw removes the eloquent extraction, which means you must use one of the database engines that supports the call inside the DB:raw call. It's fine when it's your own package but it's not a good idea in a third party package where the database driver could be using a database that doesn't support the now() call.

I think it's a more realistic expectation that your server time will be synchronized. If you have a site spanning multiple servers and their clocks are not synchronized there are lots of other issues that could happen. Why would your servers not all be using NTP to keep their time in sync?