recurring reminders keep their wall-clock hour and survive downtime (V-616)

RescheduleReminder walked the cron on the UTC instant scanReminder returns,
so a daily 09:00 Moscow reminder rescheduled to 09:00 UTC — noon the same day,
and noon every day after. And any occurrence earlier than now marked the
reminder fired, so a daemon down overnight ended the recurrence for good.

The walk now runs in the owner's location and skips past occurrences instead
of killing the reminder. Skipping and not replaying keeps the no-backlog rule
routine.DueAccepted already follows.
This commit is contained in:
2026-08-06 04:36:36 +04:00
parent 0b1efe4911
commit 13cb1903a9
2 changed files with 135 additions and 3 deletions
+31 -3
View File
@@ -205,8 +205,30 @@ func (s *Store) ListReminders(ctx context.Context, n int) ([]Reminder, error) {
// RescheduleReminder computes the next fire time for a recurring reminder and
// updates next_fire_ts. Returns ErrReminderState if the reminder is not
// recurring or not pending. If no more valid fire times exist, marks it fired.
// recurring or not pending. If the schedule yields no further fire time at all,
// marks it fired.
//
// Two things the first version got wrong, both fixed 06-08-2026 (V-616).
//
// The cron expression is a WALL CLOCK statement — "0 9 * * *" is nine in the
// morning where the owner stands — but scanReminder hands back instants in UTC,
// and robfig's Next walks the calendar in the location of the time it is given.
// Computing from a UTC instant therefore produced the next 09:00 UTC, so the
// second occurrence of a daily reminder landed one UTC offset late and stayed
// there: 12:00 for a Moscow owner. Everything is converted to loc first, which
// also makes the walk DST-correct — the schedule keeps its wall-clock hour
// across a changeover instead of drifting an hour with the offset.
//
// And a missed occurrence used to KILL the reminder: any next fire earlier than
// now marked it fired, so a daemon down overnight ended a daily standup forever.
// Occurrences in the past are skipped instead, so the reminder rolls forward to
// the first one strictly after now. Skipping and not replaying is deliberate:
// the same no-backlog rule routine.DueAccepted follows.
func (s *Store) RescheduleReminder(ctx context.Context, id int64, now time.Time) error {
return s.rescheduleReminderIn(ctx, id, now, time.Local)
}
func (s *Store) rescheduleReminderIn(ctx context.Context, id int64, now time.Time, loc *time.Location) error {
row := s.db.QueryRowContext(ctx, `
SELECT id, created_ts, fire_ts, next_fire_ts, payload, status, cron
FROM reminders WHERE id = ?`, id)
@@ -225,8 +247,14 @@ func (s *Store) RescheduleReminder(ctx context.Context, id int64, now time.Time)
if err != nil {
return fmt.Errorf("parse cron %q: %w", r.Cron, err)
}
next := sched.Next(r.NextFireTs.Add(time.Minute))
if next.IsZero() || next.Before(now) {
// Next is strictly after the time it is given, so the last fire cannot be
// returned again and no fudge minute is needed. The bound stops a schedule
// that somehow yields a non-advancing time from spinning here.
next := sched.Next(r.NextFireTs.In(loc))
for i := 0; i < 4096 && !next.IsZero() && !next.After(now); i++ {
next = sched.Next(next)
}
if next.IsZero() || !next.After(now) {
_, err = s.db.ExecContext(ctx, "UPDATE reminders SET status = 'fired' WHERE id = ?", id)
return err
}
+104
View File
@@ -0,0 +1,104 @@
package store
import (
"context"
"testing"
"time"
)
// A recurring reminder keeps its wall-clock hour in the owner's zone. The cron
// walk used to run on the UTC instant scanReminder returns, so "0 9 * * *"
// created for 09:00 Moscow rescheduled to 09:00 UTC — noon, and noon every day
// after that (V-616).
func TestRescheduleKeepsWallClockHour(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
msk := time.FixedZone("MSK", 3*60*60)
fire := time.Date(2026, 7, 1, 9, 0, 0, 0, msk)
id, err := s.CreateReminder(ctx, fire, `{"text":"стендап"}`, "0 9 * * *")
if err != nil {
t.Fatal(err)
}
if err := s.rescheduleReminderIn(ctx, id, fire, msk); err != nil {
t.Fatalf("reschedule: %v", err)
}
want := time.Date(2026, 7, 2, 9, 0, 0, 0, msk)
got := nextFire(t, s, id)
if !got.Equal(want) {
t.Fatalf("next fire: want %s, got %s", want, got.In(msk))
}
}
// A daily reminder survives a daemon that was down for days. The past
// occurrences are skipped, not replayed, and the reminder stays pending.
func TestRescheduleRollsForwardAfterDowntime(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
msk := time.FixedZone("MSK", 3*60*60)
fire := time.Date(2026, 7, 1, 9, 0, 0, 0, msk)
id, err := s.CreateReminder(ctx, fire, `{"text":"стендап"}`, "0 9 * * *")
if err != nil {
t.Fatal(err)
}
// The box comes back three days later, mid-afternoon.
now := time.Date(2026, 7, 4, 15, 0, 0, 0, msk)
if err := s.rescheduleReminderIn(ctx, id, now, msk); err != nil {
t.Fatalf("reschedule: %v", err)
}
rs, err := s.ListReminders(ctx, 10)
if err != nil || len(rs) != 1 {
t.Fatalf("list: %d reminders, %v", len(rs), err)
}
if rs[0].Status != ReminderPending {
t.Fatalf("status: want %s, got %s — downtime killed the recurrence", ReminderPending, rs[0].Status)
}
want := time.Date(2026, 7, 5, 9, 0, 0, 0, msk)
if got := nextFire(t, s, id); !got.Equal(want) {
t.Fatalf("next fire: want %s, got %s", want, got.In(msk))
}
// And exactly one fire is owed, not a backlog of four.
due, err := s.DueReminders(ctx, want.Add(time.Second))
if err != nil || len(due) != 1 {
t.Fatalf("due after roll-forward: want 1, got %d (%v)", len(due), err)
}
}
// A daily 03:00 reminder does not drift across the spring-forward changeover:
// the hour is wall clock, so the interval is 23 hours that day, not 24.
func TestRescheduleAcrossDSTKeepsHour(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
berlin, err := time.LoadLocation("Europe/Berlin")
if err != nil {
t.Skipf("tzdata unavailable: %v", err)
}
fire := time.Date(2027, 3, 27, 3, 0, 0, 0, berlin) // CET, day before the change
id, err := s.CreateReminder(ctx, fire, `{"text":"бэкап"}`, "0 3 * * *")
if err != nil {
t.Fatal(err)
}
if err := s.rescheduleReminderIn(ctx, id, fire, berlin); err != nil {
t.Fatalf("reschedule: %v", err)
}
got := nextFire(t, s, id).In(berlin)
if got.Hour() != 3 || got.Day() != 28 {
t.Fatalf("next fire: want 2027-03-28 03:00 local, got %s", got)
}
if d := got.Sub(fire); d != 23*time.Hour {
t.Fatalf("gap across spring forward: want 23h, got %s", d)
}
}
func nextFire(t *testing.T, s *Store, id int64) time.Time {
t.Helper()
var ms int64
if err := s.db.QueryRow("SELECT next_fire_ts FROM reminders WHERE id = ?", id).Scan(&ms); err != nil {
t.Fatalf("read next_fire_ts: %v", err)
}
return time.UnixMilli(ms).UTC()
}