-
-
Notifications
You must be signed in to change notification settings - Fork 958
fix: Spread feed fetch requests throughout the hour #2227
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
fix: Spread feed fetch requests throughout the hour #2227
Conversation
Previously, all RSS feeds were fetched at the top of each hour (minute 0), which could cause load spikes. This change spreads feed fetches evenly throughout the hour using a deterministic hash of the feed ID. Each feed is assigned a target minute (0-59) based on its ID hash, ensuring consistent scheduling across restarts while distributing the load evenly.
WalkthroughThe feed worker scheduling logic is modified to distribute feed enqueueing across the hour instead of synchronizing all feeds at a fixed time. A deterministic mapping from feedId to minute offset (0-59) is introduced to calculate per-feed delays, enabling staggered scheduling based on feedId. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Pre-merge checks✅ Passed checks (3 passed)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/workers/workers/feedWorker.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for type safety in all source files
Files:
apps/workers/workers/feedWorker.ts
**/*.{ts,tsx,js,jsx,json,css,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier according to project standards
Files:
apps/workers/workers/feedWorker.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lint code using oxlint and fix issues with
pnpm lint:fix
Files:
apps/workers/workers/feedWorker.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: typecheck
- GitHub Check: tests
| function getFeedMinuteOffset(feedId: string): number { | ||
| // Simple hash function: sum character codes | ||
| let hash = 0; | ||
| for (let i = 0; i < feedId.length; i++) { | ||
| hash = (hash << 5) - hash + feedId.charCodeAt(i); | ||
| hash = hash & hash; // Convert to 32-bit integer | ||
| } | ||
| // Return a minute offset between 0 and 59 | ||
| return Math.abs(hash) % 60; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bitwise operation on line 26 is a no-op.
The expression hash = hash & hash; simply returns hash unchanged. To convert to a 32-bit integer as the comment indicates, use hash = hash | 0; or hash = hash >>> 0; instead.
Apply this diff:
- hash = hash & hash; // Convert to 32-bit integer
+ hash = hash | 0; // Convert to 32-bit integer🤖 Prompt for AI Agents
In apps/workers/workers/feedWorker.ts around lines 21 to 30, the bitwise
operation `hash = hash & hash;` is a no-op but the intent is to coerce to a
32-bit integer; replace that line with a proper coercion such as `hash = hash |
0;` (for signed 32-bit) or `hash = hash >>> 0;` (for unsigned 32-bit) so the
hash becomes a 32-bit integer before taking the absolute value and modulo 60.
| const now = new Date(); | ||
| const currentMinute = now.getMinutes(); | ||
|
|
||
| for (const feed of feeds) { | ||
| const idempotencyKey = `${feed.id}-${hourlyWindow}`; | ||
| const targetMinute = getFeedMinuteOffset(feed.id); | ||
|
|
||
| // Calculate delay: if target minute has passed, schedule for next hour | ||
| let delayMinutes = targetMinute - currentMinute; | ||
| if (delayMinutes < 0) { | ||
| delayMinutes += 60; | ||
| } | ||
| const delayMs = delayMinutes * 60 * 1000; | ||
|
|
||
| logger.debug( | ||
| `[feed] Scheduling feed ${feed.id} at minute ${targetMinute} (delay: ${delayMinutes} minutes)`, | ||
| ); | ||
|
|
||
| FeedQueue.enqueue( | ||
| { | ||
| feedId: feed.id, | ||
| }, | ||
| { | ||
| idempotencyKey, | ||
| groupId: feed.userId, | ||
| delayMs, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Idempotency key can allow duplicate jobs across hour boundaries.
When the cron job runs late and a feed's target minute has already passed in the current hour, the feed is scheduled for the next hour (lines 56-59). However, the idempotencyKey is always based on hourlyWindow (the current hour, line 52), not the actual hour when the feed will execute.
Scenario:
- Cron runs late at 14:02 (instead of 14:00)
- Feed with
targetMinute = 1gets scheduled for 15:01 withidempotencyKey = "feedId-2025-12-06T14:00:00Z" - At 15:00, cron runs again and schedules the same feed for 15:01 with
idempotencyKey = "feedId-2025-12-06T15:00:00Z" - Result: Two jobs run at 15:01 for the same feed
Fix: Base the idempotency key on the actual target hour, not the current hour:
const now = new Date();
const currentMinute = now.getMinutes();
for (const feed of feeds) {
- const idempotencyKey = `${feed.id}-${hourlyWindow}`;
const targetMinute = getFeedMinuteOffset(feed.id);
// Calculate delay: if target minute has passed, schedule for next hour
let delayMinutes = targetMinute - currentMinute;
if (delayMinutes < 0) {
delayMinutes += 60;
}
const delayMs = delayMinutes * 60 * 1000;
+ // Base idempotency key on the actual hour the feed will run
+ const targetHour = new Date(now.getTime() + delayMs);
+ targetHour.setMinutes(0, 0, 0);
+ const targetHourlyWindow = targetHour.toISOString();
+ const idempotencyKey = `${feed.id}-${targetHourlyWindow}`;
+
logger.debug(
`[feed] Scheduling feed ${feed.id} at minute ${targetMinute} (delay: ${delayMinutes} minutes)`,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const now = new Date(); | |
| const currentMinute = now.getMinutes(); | |
| for (const feed of feeds) { | |
| const idempotencyKey = `${feed.id}-${hourlyWindow}`; | |
| const targetMinute = getFeedMinuteOffset(feed.id); | |
| // Calculate delay: if target minute has passed, schedule for next hour | |
| let delayMinutes = targetMinute - currentMinute; | |
| if (delayMinutes < 0) { | |
| delayMinutes += 60; | |
| } | |
| const delayMs = delayMinutes * 60 * 1000; | |
| logger.debug( | |
| `[feed] Scheduling feed ${feed.id} at minute ${targetMinute} (delay: ${delayMinutes} minutes)`, | |
| ); | |
| FeedQueue.enqueue( | |
| { | |
| feedId: feed.id, | |
| }, | |
| { | |
| idempotencyKey, | |
| groupId: feed.userId, | |
| delayMs, | |
| }, | |
| const now = new Date(); | |
| const currentMinute = now.getMinutes(); | |
| for (const feed of feeds) { | |
| const targetMinute = getFeedMinuteOffset(feed.id); | |
| // Calculate delay: if target minute has passed, schedule for next hour | |
| let delayMinutes = targetMinute - currentMinute; | |
| if (delayMinutes < 0) { | |
| delayMinutes += 60; | |
| } | |
| const delayMs = delayMinutes * 60 * 1000; | |
| // Base idempotency key on the actual hour the feed will run | |
| const targetHour = new Date(now.getTime() + delayMs); | |
| targetHour.setMinutes(0, 0, 0); | |
| const targetHourlyWindow = targetHour.toISOString(); | |
| const idempotencyKey = `${feed.id}-${targetHourlyWindow}`; | |
| logger.debug( | |
| `[feed] Scheduling feed ${feed.id} at minute ${targetMinute} (delay: ${delayMinutes} minutes)`, | |
| ); | |
| FeedQueue.enqueue( | |
| { | |
| feedId: feed.id, | |
| }, | |
| { | |
| idempotencyKey, | |
| groupId: feed.userId, | |
| delayMs, | |
| }, |
🤖 Prompt for AI Agents
In apps/workers/workers/feedWorker.ts around lines 48-74, the idempotencyKey is
built using the current hourlyWindow which can differ from the actual execution
hour when a feed is pushed to the next hour; compute the actual execution time
(e.g. executionTime = new Date(now.getTime() + delayMs)) or derive the
targetHour by adding one hour when delayMinutes < 0, then build the
idempotencyKey from that execution hour (formatted the same way as hourlyWindow)
instead of the current hourlyWindow so jobs scheduled across an hour boundary
share a stable, correct idempotency key.
Previously, all RSS feeds were fetched at the top of each hour (minute 0), which could cause load spikes. This change spreads feed fetches evenly throughout the hour using a deterministic hash of the feed ID.
Each feed is assigned a target minute (0-59) based on its ID hash, ensuring consistent scheduling across restarts while distributing the load evenly.