Skip to content

feat: add jitter wrappers to prevent thundering herd#258

Merged
CybotTM merged 1 commit intomainfrom
feat/jitter-support
Dec 15, 2025
Merged

feat: add jitter wrappers to prevent thundering herd#258
CybotTM merged 1 commit intomainfrom
feat/jitter-support

Conversation

@CybotTM
Copy link
Member

@CybotTM CybotTM commented Dec 15, 2025

Summary

Adds jitter support to prevent thundering herd problems when many jobs are scheduled at the same time.

Closes #227

  • Adds Jitter(maxJitter time.Duration) JobWrapper - adds random delay in [0, maxJitter)
  • Adds JitterWithLogger(logger, maxJitter) - same with delay logging for observability
  • Zero or negative maxJitter disables jitter (no-op behavior)
  • Uses math/rand/v2 from Go stdlib (no external dependency)

Implementation Approach

This implements jitter using the existing JobWrapper pattern rather than the upstream approach (robfig#525) that adds 4 new methods to the Cron type. Benefits:

  • Composable: Works seamlessly with other wrappers (Recover, SkipIfStillRunning, etc.)
  • Consistent API: Follows established patterns in this codebase
  • Flexible: Can be applied globally via WithChain or per-job via NewChain().Then()
  • Zero overhead: Disabled jitter adds no delay

Usage Examples

// Global jitter for all jobs
c := cron.New(cron.WithChain(
    cron.Recover(logger),
    cron.Jitter(30 * time.Second),
))

// Per-job jitter
job := cron.NewChain(cron.Jitter(5 * time.Second)).Then(myJob)
c.Schedule(cron.Every(time.Hour), job)

Test plan

  • Tests verify delay is within expected range
  • Tests verify zero/negative maxJitter disables jitter
  • Tests verify uniform distribution across range
  • Tests verify composition with other wrappers
  • Tests verify logging behavior
  • All existing tests pass
  • Pre-commit hooks pass (lint, build, security scan)

Add Jitter() and JitterWithLogger() JobWrappers that add a random delay
before job execution to prevent thundering herd problems when many jobs
are scheduled at the same time.

Features:
- Jitter(maxJitter): Adds random delay in [0, maxJitter) range
- JitterWithLogger(logger, maxJitter): Same with delay logging
- Zero or negative maxJitter disables jitter (no-op)
- Uses math/rand/v2 from Go stdlib (no external dependency)

This addresses issue #227, implementing jitter using the existing
JobWrapper pattern rather than the upstream approach of adding
multiple new methods to the Cron type.
Copilot AI review requested due to automatic review settings December 15, 2025 14:08
@github-actions
Copy link

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@CybotTM CybotTM merged commit 38d6cd5 into main Dec 15, 2025
18 checks passed
@CybotTM CybotTM deleted the feat/jitter-support branch December 15, 2025 14:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds jitter support to prevent thundering herd problems when multiple jobs execute simultaneously. The implementation uses a composable JobWrapper pattern that integrates seamlessly with existing wrappers.

Key changes:

  • Adds Jitter(maxJitter) and JitterWithLogger(logger, maxJitter) wrapper functions that add random delays before job execution
  • Uses math/rand/v2 for generating uniformly distributed delays in [0, maxJitter)
  • Zero or negative maxJitter values disable jitter for zero overhead

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
chain.go Implements Jitter and JitterWithLogger wrapper functions with uniform random delay distribution
chain_test.go Adds comprehensive tests covering delay behavior, edge cases, composition with other wrappers, and logging
doc.go Updates documentation to include jitter in the list of available wrappers
example_test.go Adds three example functions demonstrating global jitter, per-job jitter, and jitter with logging

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +847 to +867
panicJob := FuncJob(func() {
recovered = false
panic("test panic")
})

wrappedPanicJob := NewChain(
Recover(DiscardLogger),
Jitter(10*time.Millisecond),
).Then(panicJob)

// Should not panic due to Recover wrapper
func() {
defer func() {
if r := recover(); r != nil {
t.Errorf("panic was not recovered: %v", r)
} else {
recovered = true
}
}()
wrappedPanicJob.Run()
}()
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The test logic for verifying panic recovery is flawed. The 'recovered' variable is set to false inside the panicJob function, but this assignment never executes because the panic occurs immediately. The defer function in the test then sets 'recovered = true' in the else branch when no panic reaches it (since the Recover wrapper catches it). This doesn't actually verify that the Recover wrapper caught the panic - it only verifies that the test function itself didn't receive a panic.

To properly test this, you should verify that the job panicked and was recovered by checking logs or using a different mechanism to track recovery.

Copilot uses AI. Check for mistakes.
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.

feat: Add jitter/randomized delay support

1 participant