Skip to content

Refactor leaderboard scheduler to prevent race conditions.#2489

Open
sesposito wants to merge 2 commits intomasterfrom
spe/fix-tournament-end-hook
Open

Refactor leaderboard scheduler to prevent race conditions.#2489
sesposito wants to merge 2 commits intomasterfrom
spe/fix-tournament-end-hook

Conversation

@sesposito
Copy link
Copy Markdown
Member

@sesposito sesposito commented Apr 6, 2026

The new design ensures a main loop controls all the scheduling logic, resolving some race conditions which could prevent the hooks to fire in some conditions.

The new design ensures a main loop controls all the scheduling
logic, this way there's no chance of race conditions when
a expiry/end hook creates a new leaderboard from within itself.
Copy link
Copy Markdown

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

Refactors the leaderboard/tournament scheduler to use a single scheduling loop (instead of multiple timers + mutex) to reduce race conditions and ensure hook execution is coordinated from one place.

Changes:

  • Replaced per-deadline timers with a centralized scheduleLoop driven by an updateCh wake signal.
  • Split scheduling into computeNext (deadline calculation) and processHooks (execution/enqueueing).
  • Hardened tournament end lookup error handling using errors.Is(err, sql.ErrNoRows).

Reviewed changes

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

File Description
server/leaderboard_scheduler.go Replaces multi-timer scheduling with a single loop + update channel; reorganizes hook processing and queueing.
server/leaderboard_scheduler_test.go Removes obsolete comments related to the prior timer race reproduction mechanism.

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


earliestEndActive := int64(-1)
earliestExpiry := int64(-1)
func (ls *LocalLeaderboardScheduler) processHooks(ts time.Time, endActiveTs, expiryTs int64, endActiveIds, expiryIds []string) {
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

processHooks can still enqueue and run callbacks after Pause() if the timer fires concurrently with the pause transition (the scheduling loop checks active only at the top of the loop). Previously queueEndActiveElapse/queueExpiryElapse guarded on active.Load(). Consider adding an if ls.active.Load() != 1 { return } check at the start of processHooks (or before each processHooks call) to preserve pause semantics and avoid hooks firing while paused.

Suggested change
func (ls *LocalLeaderboardScheduler) processHooks(ts time.Time, endActiveTs, expiryTs int64, endActiveIds, expiryIds []string) {
func (ls *LocalLeaderboardScheduler) processHooks(ts time.Time, endActiveTs, expiryTs int64, endActiveIds, expiryIds []string) {
if ls.active.Load() != 1 {
return
}

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.

2 participants