Skip to content

fix: use configured Location for all schedule evaluations #2312#2313

Merged
yohamta0 merged 1 commit into
dagucloud:mainfrom
four-flames:feat/2312-fix-timezone-asymmetry
Jun 21, 2026
Merged

fix: use configured Location for all schedule evaluations #2312#2313
yohamta0 merged 1 commit into
dagucloud:mainfrom
four-flames:feat/2312-fix-timezone-asymmetry

Conversation

@four-bytes-robby

@four-bytes-robby four-bytes-robby commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Closes #2312

Problem

The scheduler had a timezone asymmetry: start schedules and one-off schedules used raw now (system clock time) while stop/restart schedules converted now to the configured cfg.Location. When DAGU_TZ is configured to a different timezone than the server's local time, start schedules, one-off schedules, and catchup computations fire at the wrong wall-clock time.

Root Cause

The misleading comment in tick_planner.go claimed robfig/cron applies the schedule's timezone internally — but the 5-field standard parser has no timezone support (no CRON_TZ=).

Additionally, os.Setenv("TZ", cfg.TZ) partially masks the bug by making time.Local == cfg.Location, but any code path that creates times without going through time.Now() (persisted watermarks, replay times, concurrent access) carries the wrong location.

Fix

Use now.In(tp.cfg.Location) consistently for ALL schedule evaluations:

Location Change
Plan() start schedules nowevalTime (now.In(cfg.Location))
Plan() one-off schedules nowevalTime
Init() one-off reconciliation tp.cfg.Clock()tp.cfg.Clock().In(tp.cfg.Location)
recomputeBuffer() catchup tp.cfg.Clock()tp.cfg.Clock().In(tp.cfg.Location)

evalTime is now computed once at the top of Plan() and used in all four schedule loops (start, stop, restart, one-off).

Impact

  • Remote DAGs with timezone-aware schedules now evaluate correctly
  • Consistent with existing stop/restart behavior
  • No behavior change when cfg.Location == time.Local
  • All existing scheduler tests pass

Summary by cubic

Fix a timezone mismatch in the scheduler. All schedules now use the configured location, so triggers fire at the expected wall-clock time when DAGU_TZ differs from the server timezone. Fixes #2312.

  • Bug Fixes
    • Use a single evalTime := now.In(tp.cfg.Location) for all checks in Plan() (cron start and one-off).
    • Convert tp.cfg.Clock() to tp.cfg.Location in Init() reconciliation and recomputeBuffer() catchup.
    • Avoid relying on robfig/cron 5-field parser for timezone handling; no change when cfg.Location == time.Local.

Written for commit 7249a67. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Fixed scheduler timing evaluation to consistently apply configured timezone settings across all schedule due-time checks. Resolves potential timing misalignment issues when using non-UTC timezones for schedule execution.

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b88fc414-5d0a-4276-9546-3cdd63bff569

📥 Commits

Reviewing files that changed from the base of the PR and between 9b6a7ff and 7249a67.

📒 Files selected for processing (1)
  • internal/service/scheduler/tick_planner.go

📝 Walkthrough

Walkthrough

All schedule evaluation points in tick_planner.go are updated to use tp.cfg.Clock().In(tp.cfg.Location) or now.In(tp.cfg.Location) consistently. Previously, start schedules, one-off schedules, and catchup recomputation used raw clock time while stop/restart schedules already used the configured location.

Changes

Scheduler Location Consistency Fix

Layer / File(s) Summary
Init and recomputeBuffer location normalization
internal/service/scheduler/tick_planner.go
Init computes observedAt via tp.cfg.Clock().In(tp.cfg.Location) and recomputeBuffer computes its local now the same way, replacing raw clock references in both.
Plan: unified evalTime across all schedule types
internal/service/scheduler/tick_planner.go
Plan introduces evalTime := now.In(tp.cfg.Location) and applies it to one-off readiness checks, cron scheduleDueAt calls, and stop/restart evaluation, removing the prior in-block re-localization in the stop/restart block.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main fix: applying the configured Location consistently across all schedule evaluations in the scheduler.
Description check ✅ Passed The description comprehensively covers the problem, root cause, fix, and impact with clear detail and issue reference, though formal checklist items are absent.
Linked Issues check ✅ Passed The PR fully addresses issue #2312 by applying configured Location consistently across all four schedule evaluation contexts (start, one-off, init reconciliation, and catchup).
Out of Scope Changes check ✅ Passed All code changes in tick_planner.go are directly aligned with fixing the timezone asymmetry issue; no unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yohamta0 yohamta0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks !

@yohamta0 yohamta0 merged commit 17b9442 into dagucloud:main Jun 21, 2026
11 checks passed
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

Successfully merging this pull request may close these issues.

fix: timezone asymmetry in scheduler — start schedules ignore configured Location

2 participants