grandnode / grandnode2

Open-Source eCommerce Platform on .NET Core, MongoDB, AWS DocumentDB, Azure CosmosDB, LiteDB & Vue.js
https://grandnode.com/
GNU General Public License v3.0
1.06k stars 435 forks source link

BackgroundServiceTask: timeInterval computation is wrong #425

Closed serlooks closed 12 months ago

serlooks commented 1 year ago

The line: https://github.com/grandnode/grandnode2/blob/main/src/Web/Grand.Web.Common/Infrastructure/BackgroundServiceTask.cs#L57 is wrong.

If a task is executed before it should, this check makes sure to delay it further until it's proper execution time, however this line does not compute it properly. The computed value later goes to do Task.Delay(timeInterval).

Few debug exports:

UTC Now LastStart Base Interval LastStart + Interval Computed new Interval What it should be
06.09.2023 09:27:03 06.09.2023 08:44:57 60 06.09.2023 09:44:57 42 17
06.09.2023 09:27:06 05.09.2023 11:51:52 1440 06.09.2023 11:51:52 1295 144

The line should be replaced either with: timeInterval = (int)(task.LastStartUtc.Value.AddMinutes(task.TimeInterval) - datetimenow).TotalMinutes or timeInterval = task.TimeInterval - (int)(datetimenow - task.LastStartUtc.Value).TotalMinutes;

which will put the task to sleep for the remaining time.
This was discovered when we noticed our tasks running at an interval of 119 minutes instead of 60. The task would attempt to execute maybe a few seconds before it should, and current implementation reschedules it in another 59 minutes.

Please verify this and correct me if I'm wrong.

Best Regards, Luka

KrzysztofPajak commented 12 months ago

@serlooks Thanks