Skip to content

Commit f5c56e2

Browse files
claudeenko
authored andcommitted
docs: Add idempotency guard to notification scheduler
Add last_notified_date column to notification_channels table and update scheduler pseudo-code with atomic check-and-set to prevent duplicate digests when multiple backend instances run or when the job re-fires within the same minute. Failed deliveries leave last_notified_date unchanged so the channel is retried on the next tick. Addresses PR review comment about missing idempotency protection. https://claude.ai/code/session_01VP4wi9RRFsCDgBbbY5xkCL
1 parent 87f49b9 commit f5c56e2

File tree

1 file changed

+31
-3
lines changed

1 file changed

+31
-3
lines changed

project-management/issues/upcoming-dates-notifications.md

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ CREATE TABLE IF NOT EXISTS system.notification_channels (
136136
-- Per-channel notification preferences
137137
lookahead_days INTEGER NOT NULL DEFAULT 7 CHECK (lookahead_days BETWEEN 1 AND 30),
138138
notify_time TIME NOT NULL DEFAULT '08:00:00',
139+
last_notified_date DATE,
139140

140141
created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
141142
updated_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
@@ -332,8 +333,15 @@ The job runs every minute and checks which enabled channels have a `notify_time`
332333
cron.schedule('* * * * *', async () => {
333334
const now = new Date();
334335
const currentTime = `${String(now.getUTCHours()).padStart(2, '0')}:${String(now.getUTCMinutes()).padStart(2, '0')}`;
336+
const todayUtc = now.toISOString().slice(0, 10); // 'YYYY-MM-DD'
335337

336-
const dueChannels = await getEnabledChannelsDueAt.run({ notifyTime: currentTime }, pool);
338+
// Idempotency: only select channels that haven't already been notified today.
339+
// The WHERE clause includes `(last_notified_date IS NULL OR last_notified_date < :today)`
340+
// so duplicate runs within the same minute or across multiple instances are safe.
341+
const dueChannels = await getEnabledChannelsDueAt.run(
342+
{ notifyTime: currentTime, today: todayUtc },
343+
pool,
344+
);
337345

338346
for (const channel of dueChannels) {
339347
try {
@@ -343,22 +351,42 @@ cron.schedule('* * * * *', async () => {
343351
limitCount: 50,
344352
}, pool);
345353

346-
if (upcomingDates.length === 0) continue;
354+
if (upcomingDates.length === 0) {
355+
// No upcoming dates — still mark as notified so we don't re-check every minute
356+
await markChannelNotified.run({ channelId: channel.id, today: todayUtc }, pool);
357+
continue;
358+
}
347359

348360
const locale = channel.user_language ?? 'en';
349361
const message = formatNotificationMessage(upcomingDates, locale);
350362
await dispatch(channel, message);
363+
364+
// Atomically set last_notified_date to today after successful dispatch
365+
await markChannelNotified.run({ channelId: channel.id, today: todayUtc }, pool);
351366
logger.info({ channelExternalId: channel.external_id }, 'Notification dispatched');
352367
} catch (error) {
353368
const err = toError(error);
354369
logger.error({ err, channelExternalId: channel.external_id }, 'Failed to dispatch notification');
355370
Sentry.captureException(err);
371+
// last_notified_date is NOT updated on failure, so the channel will be retried next minute
356372
}
357373
}
358374
});
359375
```
360376

361-
A new PgTyped query `getEnabledChannelsDueAt` is needed that selects from `system.notification_channels` where `is_enabled = true` and `notify_time = :notifyTime`, joining to `auth.users` to retrieve `user_external_id` and the user's `language` preference (extracted from `preferences->'language'`, defaulting to `'en'`).
377+
**Idempotency:** The `last_notified_date` column ensures each channel receives at most one digest per calendar day (UTC). The `getEnabledChannelsDueAt` query filters out channels where `last_notified_date` already matches today's date, making the scheduler safe to run across multiple backend instances. A separate `markChannelNotified` PgTyped query updates the column:
378+
379+
```sql
380+
/* @name MarkChannelNotified */
381+
UPDATE system.notification_channels
382+
SET last_notified_date = :today
383+
WHERE id = :channelId
384+
AND (last_notified_date IS NULL OR last_notified_date < :today::date);
385+
```
386+
387+
The `WHERE` guard in the UPDATE makes the mark itself idempotent — concurrent instances racing to mark the same channel will not conflict. If dispatch fails, `last_notified_date` is left unchanged so the scheduler retries on the next minute tick until either delivery succeeds or the day rolls over.
388+
389+
A new PgTyped query `getEnabledChannelsDueAt` is needed that selects from `system.notification_channels` where `is_enabled = true`, `notify_time = :notifyTime`, and `(last_notified_date IS NULL OR last_notified_date < :today::date)`, joining to `auth.users` to retrieve `user_external_id` and the user's `language` preference (extracted from `preferences->'language'`, defaulting to `'en'`). The `last_notified_date` filter is the idempotency guard that prevents duplicate notifications.
362390

363391
**Important:** The scheduler pseudo-code above calls `getUpcomingDates` with `maxDays` and `limitCount` parameters. The existing `GetUpcomingDates` query in `friend-dates.sql` already accepts these exact parameters (`maxDays` for the lookahead window, `limitCount` for the result limit), so no modification to the existing query is needed. The scheduler reuses it as-is — the only difference from the frontend dashboard usage is that different values are passed at call time (per-channel `lookahead_days` rather than a hardcoded default). A new query is **not** required; the existing one is fully compatible.
364392

0 commit comments

Comments
 (0)