Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cron parser fails to update offset when calling last after daylight saving time change in US #6223

Open
bhb opened this issue Mar 11, 2024 · 1 comment

Comments

@bhb
Copy link

bhb commented Mar 11, 2024

Ruby version: 3.2.2
Rails version: 7.0.8.1
Sidekiq / Pro / Enterprise version(s): 7.2.1/ 7.2.0 / 7.2.1

We recently "lost an hour" due to daylight savings time. The next day, I noticed that Sidekiq::CronParser reports the last time a job was run was roughly 24 hours ago, but in reality, Sidekiq ran the job 23 hours ago (which is correct).

I realize you are using a fork of this gem, so if you prefer, I can file the bug on that repo.

Repro:

parsed_schedule = Sidekiq::CronParser.new("0 1 * * *") # daily, 1AM
time = Time.zone.parse("2024-03-11 01:00:00 -0600")    # day after DST change
last_time = parsed_schedule.last(time)

(time - last_time) / 1.hour # 24 hours

# Actual UTC times from my logs (this is correct!)
x1 = Time.zone.parse("2024-03-10 01:00:00 -0700")
x2 = Time.zone.parse("2024-03-11 01:00:00 -0600")
(x2 - x1) / 1.hours # 22.9 hours (almost 23, not 24 hours)

# Note the difference between last_time and x1 (the offset is different)
last_time.to_s # "2024-03-10 01:00:00 -0600"
x1.to_s        # "2024-03-10 01:00:00 -0700"

Actual: Sidekiq behavior is correct, but CronParser#last incorrectly reports that the previous scheduled time is 24 hours ago.
Expected: Sidekiq behavior stays the same, but CronParser#last correctly reports the previous scheduled time was 23 hours ago.

@mperham
Copy link
Collaborator

mperham commented Mar 11, 2024

I think the history records are probably more reliable for getting runtimes.

I don’t plan to ever touch the cron time handling code again. Unless there’s a bug leading to Bad Things Happening, I’m going to close as Won’t Fix. It’s impossible to reliably test all the various edge cases so I’d prefer to leave it as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants