feat(model): add jitter to next_check_at scheduling to avoid thundering herd #3821
Conversation
|
I think that this should be good, maybe add a variable for the max jitter value |
There was a problem hiding this comment.
Pull Request Overview
This PR adds randomized jitter to the scheduling of feed checks to prevent thundering herd problems when multiple feeds are checked simultaneously. The implementation adds 0-10 minutes of random delay to the calculated check interval while ensuring the final interval still respects the configured maximum limits.
- Added random jitter (0-10 minutes) to feed check scheduling intervals
- Updated test helper to account for the additional jitter when validating check times
- Applied interval clamping after jitter addition to maintain configuration limits
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/model/feed.go | Implements jitter logic in ScheduleNextCheck method with random delay and re-clamping |
| internal/model/feed_test.go | Updates test helper to allow for jitter tolerance in interval validation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
internal/model/feed.go
Outdated
| if jitterMax > 0 { | ||
| jitter := time.Duration(rand.Int63n(int64(jitterMax + 1))) | ||
| interval += jitter | ||
|
|
||
| // Re-apply max clamping after jitter to avoid exceeding configured caps. | ||
| switch config.Opts.PollingScheduler() { | ||
| case SchedulerRoundRobin: | ||
| interval = min(interval, config.Opts.SchedulerRoundRobinMaxInterval()) | ||
| case SchedulerEntryFrequency: | ||
| interval = min(interval, config.Opts.SchedulerEntryFrequencyMaxInterval()) | ||
| } |
There was a problem hiding this comment.
The condition jitterMax > 0 is unnecessary since jitterMax is a constant value of 10 minutes. This check can be removed to simplify the code.
| if jitterMax > 0 { | |
| jitter := time.Duration(rand.Int63n(int64(jitterMax + 1))) | |
| interval += jitter | |
| // Re-apply max clamping after jitter to avoid exceeding configured caps. | |
| switch config.Opts.PollingScheduler() { | |
| case SchedulerRoundRobin: | |
| interval = min(interval, config.Opts.SchedulerRoundRobinMaxInterval()) | |
| case SchedulerEntryFrequency: | |
| interval = min(interval, config.Opts.SchedulerEntryFrequencyMaxInterval()) | |
| } | |
| jitter := time.Duration(rand.Int63n(int64(jitterMax + 1))) | |
| interval += jitter | |
| // Re-apply max clamping after jitter to avoid exceeding configured caps. | |
| switch config.Opts.PollingScheduler() { | |
| case SchedulerRoundRobin: | |
| interval = min(interval, config.Opts.SchedulerRoundRobinMaxInterval()) | |
| case SchedulerEntryFrequency: | |
| interval = min(interval, config.Opts.SchedulerEntryFrequencyMaxInterval()) |
fguillot
left a comment
There was a problem hiding this comment.
Beside the comment opened by Copilot, is it possible to avoid the duplication of code regarding the switch/case?
feat: add config variable for polling jitter
description: ScheduleNextCheck now adds random jitter of 0–10 minutes to the calculated interval and re-limits the value to the upper limits of the configuration. This reduces the likelihood of massive simultaneous requests to a single host after failed updates and helps bypass frequency limits.
testing: updated check_target_interval to allow for an upper limit of +10m jitter; local linters on changed files without errors.
breaking changes: none