From 4db3001c43f67a5aeaf9a9483d42762a58d47a2e Mon Sep 17 00:00:00 2001
From: Isaac Levy <isaac.r.levy@gmail.com>
Date: Wed, 18 Sep 2024 18:05:36 -0400
Subject: [PATCH 1/5] Refactor Tourney classes

- Move recently created complex logic into its own object
- Add a deterministic tourney stagger method
- Add stagger to Plan objects, rather than Tournaments to
  be more easily testable.
---
 modules/tournament/src/main/PlanBuilder.scala | 194 ++++++++++++++++++
 modules/tournament/src/main/Schedule.scala    |  84 +-------
 modules/tournament/src/main/Tournament.scala  |   5 +-
 .../src/main/TournamentScheduler.scala        |  35 +---
 .../tournament/src/test/PlanBuilderTest.scala |  65 ++++++
 .../src/test/ScheduleTestHelpers.scala        |   4 +-
 .../tournament/src/test/SchedulerTest.scala   |   4 +-
 7 files changed, 274 insertions(+), 117 deletions(-)
 create mode 100644 modules/tournament/src/main/PlanBuilder.scala
 create mode 100644 modules/tournament/src/test/PlanBuilderTest.scala

diff --git a/modules/tournament/src/main/PlanBuilder.scala b/modules/tournament/src/main/PlanBuilder.scala
new file mode 100644
index 000000000000..317f0be80cc6
--- /dev/null
+++ b/modules/tournament/src/main/PlanBuilder.scala
@@ -0,0 +1,194 @@
+package lila.tournament
+
+import scala.collection.mutable
+import scala.collection.SortedSet
+// For comparing Instants
+import scala.math.Ordering.Implicits.infixOrderingOps
+
+import Schedule.Plan
+
+object PlanBuilder:
+  /** Max window for daily or better schedules to be considered overlapping (i.e. if one starts within X hrs
+    * of the other ending). Using 11.5 hrs here ensures that at least one daily is always cancelled for the
+    * more important event. But, if a higher importance tourney is scheduled nearly opposite of the daily
+    * (i.e. 11:00 for a monthly and 00:00 for its daily), two dailies will be cancelled... so don't do this!
+    */
+  private[tournament] val SCHEDULE_DAILY_OVERLAP_MINS = 690 // 11.5 hours
+
+  // 2/3 of a min ensures good spacing from tourneys starting the next minute, which aren't explicitly
+  // considered when calculating ideal stagger. It also keeps at least or better spacing than the prior
+  // random [0, 60) sec stagger.
+  private[tournament] val MAX_STAGGER_MS = 40_000
+
+  private[tournament] abstract class ScheduleWithInterval:
+    def schedule: Schedule
+    def duration: java.time.Duration
+    def startsAt: Instant
+
+    val endsAt = startsAt.plus(duration)
+
+    def interval = TimeInterval(startsAt, endsAt)
+
+    def overlaps(other: ScheduleWithInterval) = interval.overlaps(other.interval)
+
+    // Note: must be kept in sync with [[SchedulerTestHelpers.ExperimentalPruner.pruneConflictsFailOnUsurp]]
+    // In particular, pruneConflictsFailOnUsurp filters tourneys that couldn't possibly conflict based
+    // on their hours -- so the same logic (overlaps and daily windows) must be used here.
+    def conflictsWith(si2: ScheduleWithInterval) =
+      val s1 = schedule
+      val s2 = si2.schedule
+      s1.variant == s2.variant && (
+        // prevent daily && weekly within X hours of each other
+        if s1.freq.isDailyOrBetter && s2.freq.isDailyOrBetter && s1.sameSpeed(s2) then
+          si2.startsAt.minusMinutes(SCHEDULE_DAILY_OVERLAP_MINS).isBefore(endsAt) &&
+          startsAt.minusMinutes(SCHEDULE_DAILY_OVERLAP_MINS).isBefore(si2.endsAt)
+        else
+          (
+            s1.variant.exotic ||  // overlapping exotic variant
+              s1.hasMaxRating ||  // overlapping same rating limit
+              s1.similarSpeed(s2) // overlapping similar
+          ) && s1.similarConditions(s2) && overlaps(si2)
+      )
+
+    /** Kept in sync with [[conflictsWithFailOnUsurp]].
+      */
+    def conflictsWith(scheds: Iterable[ScheduleWithInterval]): Boolean =
+      scheds.exists(conflictsWith)
+
+    /** Kept in sync with [[conflictsWith]].
+      *
+      * Raises an exception if a tourney is incorrectly usurped.
+      */
+    @throws[IllegalStateException]("if a tourney is incorrectly usurped")
+    def conflictsWithFailOnUsurp(scheds: Iterable[ScheduleWithInterval]) =
+      val conflicts   = scheds.filter(conflictsWith).toSeq
+      val okConflicts = conflicts.filter(_.schedule.freq >= schedule.freq)
+      if conflicts.nonEmpty && okConflicts.isEmpty then
+        throw new IllegalStateException(s"Schedule [$schedule] usurped by ${conflicts}")
+      conflicts.nonEmpty
+
+  private final case class ConcreteSchedule(
+      val schedule: Schedule,
+      val startsAt: Instant,
+      val duration: java.time.Duration
+  ) extends ScheduleWithInterval
+
+  private[tournament] def filterAndStaggerPlans(
+      existingTourneys: Iterable[Tournament],
+      plans: Iterable[Plan]
+  ): List[Plan] =
+    // Prune plans using the unstaggered scheduled start time.
+    val existingWithScheduledStart = existingTourneys.flatMap { t =>
+      // Ignore tournaments with schedule=None - they never conflict.
+      t.schedule.map { s => ConcreteSchedule(s, s.atInstant, t.duration) }
+    }
+
+    val prunedPlans = pruneConflicts(existingWithScheduledStart, plans)
+
+    // Unlike pruning, stagger plans even against Tourneys with schedule=None.
+    plansWithStagger(existingTourneys, prunedPlans)
+
+  /** Given a list of existing schedules and a list of possible new plans, returns a subset of the possible
+    * plans that do not conflict with either the existing schedules or with themselves. Intended to produce
+    * identical output to [[SchedulerTestHelpers.ExperimentalPruner.pruneConflictsFailOnUsurp]], but this
+    * variant is more readable and has lower potential for bugs.
+    */
+  private[tournament] def pruneConflicts[A <: ScheduleWithInterval](
+      existingSchedules: Iterable[ScheduleWithInterval],
+      possibleNewPlans: Iterable[A]
+  ): List[A] =
+    var allPlannedSchedules = existingSchedules.toList
+    possibleNewPlans
+      .foldLeft(Nil): (newPlans, p) =>
+        if p.conflictsWith(allPlannedSchedules) then newPlans
+        else
+          allPlannedSchedules ::= p
+          p :: newPlans
+      .reverse
+
+  /** Given a plan, return an adjusted start time that minimizes conflicts with existing events.
+    *
+    * This method assumes that no event starts immediately before plan or after the max stagger, in terms of
+    * impacting server performance by concurrent events.
+    */
+  private[tournament] def getAdjustedStart(
+      plan: Plan,
+      existingEvents: SortedSet[Instant],
+      maxStaggerMs: Long
+  ): Instant =
+
+    val originalStart   = plan.startsAt
+    val originalStartMs = originalStart.toEpochMilli
+    val maxConflictAt   = originalStart.plusMillis(maxStaggerMs)
+
+    // Find all events that start at a similar time to the plan.
+    val offsetsWithSimilarStart = existingEvents
+      .iteratorFrom(originalStart)
+      .takeWhile(_ <= maxConflictAt)
+      // Shift events times to be relative to original plan, to avoid loss of precision
+      .map(s => (s.toEpochMilli - originalStartMs).toFloat)
+      .toSeq
+
+    val staggerMs = findMinimalGoodSlot(0f, maxStaggerMs, offsetsWithSimilarStart)
+    originalStart.plusMillis(staggerMs.toLong)
+
+  /** Given existing tourneys and possible new plans, returns new Plan objects that are staggered to avoid
+    * starting at the exact same time as other plans or tourneys. Does NOT filter for conflicts.
+    */
+  private[tournament] def plansWithStagger(
+      existingTourneys: Iterable[Tournament],
+      plans: Iterable[Plan]
+  ): List[Plan] =
+    // Determine stagger using the actual (staggered) start time of existing and new tourneys.
+    val allTourneyStarts = mutable.TreeSet.from(existingTourneys.map(_.startsAt))
+
+    plans
+      .foldLeft(Nil): (allAdjustedPlans, plan) =>
+        val adjustedStart = getAdjustedStart(plan, allTourneyStarts, MAX_STAGGER_MS)
+        allTourneyStarts += adjustedStart
+        plan.copy(startsAt = adjustedStart) :: allAdjustedPlans
+      .reverse
+
+  /** This method is used find a good stagger value for a new tournament. We want stagger as low as possible,
+    * because that means tourneys start sooner, but we also want tournaments to be spaced out to avoid server
+    * DDOS.
+    *
+    * The method uses Floats. Assuming the original plans use whole numbers (of seconds), successive staggers
+    * will be whole multiples of a negative power of 2, and so are be exactly representable as a Float. Neat!
+    *
+    * Behavior is only loosely defined when the length of [low, hi] approaches or exceeds [[Float.MaxValue]]
+    * or when the range is centered around a large number and loses precision. Neither of these scenarios is
+    * how the function is expected to be used in practice.
+    *
+    * @param hi
+    *   must be >= low
+    * @param sortedExisting
+    *   must be sorted and contain only values in [low, hi]
+    *
+    * @return
+    *   the lowest value in [low, hi] with maximal distance to elts of sortedExisting
+    */
+  private[tournament] def findMinimalGoodSlot(low: Float, hi: Float, sortedExisting: Iterable[Float]): Float =
+    // Computations use doubles to avoid loss of precision and rounding errors.
+    if sortedExisting.isEmpty then low
+    else
+      val iter    = sortedExisting.iterator
+      var prevElt = iter.next.toDouble
+      // Fake gap low to check later (i.e. maxGapLow < low)
+      var maxGapLow = Double.NegativeInfinity
+      // nothing is at low element so gap is equiv to 2x
+      var maxGapLen = (prevElt - low) * 2.0
+      while iter.hasNext do
+        val elt = iter.next.toDouble
+        if elt - prevElt > maxGapLen then
+          maxGapLow = prevElt
+          maxGapLen = elt - prevElt
+        prevElt = elt
+      // Use hi if it's strictly better than all other gaps. Since nothing is at hi, gap is equiv
+      // to 2x maxGapLen.
+      if (hi - prevElt) * 2.0 > maxGapLen then hi
+      // Else, use the first best slot, whose first candidate is low. Using a special case for low
+      // guarantees we always return in the interval [low, high], and don't have to be quite as
+      // vigilant with floating point rounding errors.
+      else if maxGapLow < low then low
+      else (maxGapLow + maxGapLen * 0.5).toFloat
diff --git a/modules/tournament/src/main/Schedule.scala b/modules/tournament/src/main/Schedule.scala
index 7238e9c134a8..80993c302ba4 100644
--- a/modules/tournament/src/main/Schedule.scala
+++ b/modules/tournament/src/main/Schedule.scala
@@ -127,8 +127,8 @@ case class Schedule(
 
   def perfKey: PerfKey = PerfKey.byVariant(variant) | Schedule.Speed.toPerfKey(speed)
 
-  def plan                                  = Schedule.Plan(this, None)
-  def plan(build: Tournament => Tournament) = Schedule.Plan(this, build.some)
+  def plan                                  = Schedule.Plan(this, atInstant, None)
+  def plan(build: Tournament => Tournament) = Schedule.Plan(this, atInstant, build.some)
 
   override def toString =
     s"${atInstant} $freq ${variant.key} ${speed.key}(${Schedule.clockFor(this)}) $conditions $position"
@@ -144,73 +144,17 @@ object Schedule:
       at = tour.startsAt.dateTime
     )
 
-  /** Max window for daily or better schedules to be considered overlapping (i.e. if one starts within X hrs
-    * of the other ending). Using 11.5 hrs here ensures that at least one daily is always cancelled for the
-    * more important event. But, if a higher importance tourney is scheduled nearly opposite of the daily
-    * (i.e. 11:00 for a monthly and 00:00 for its daily), two dailies will be cancelled... so don't do this!
-    */
-  private[tournament] val SCHEDULE_DAILY_OVERLAP_MINS = 690 // 11.5 * 60
-
-  private[tournament] trait ScheduleWithInterval:
-    def schedule: Schedule
-    def startsAt: Instant
-    def duration: java.time.Duration
-
-    def endsAt = startsAt.plus(duration)
-
-    def interval = TimeInterval(startsAt, duration)
-
-    def overlaps(other: ScheduleWithInterval) = interval.overlaps(other.interval)
-
-    // Note: must be kept in sync with [[SchedulerTestHelpers.ExperimentalPruner.pruneConflictsFailOnUsurp]]
-    // In particular, pruneConflictsFailOnUsurp filters tourneys that couldn't possibly conflict based
-    // on their hours -- so the same logic (overlaps and daily windows) must be used here.
-    def conflictsWith(si2: ScheduleWithInterval) =
-      val s1 = schedule
-      val s2 = si2.schedule
-      s1.variant == s2.variant && (
-        // prevent daily && weekly within X hours of each other
-        if s1.freq.isDailyOrBetter && s2.freq.isDailyOrBetter && s1.sameSpeed(s2) then
-          si2.startsAt.minusMinutes(SCHEDULE_DAILY_OVERLAP_MINS).isBefore(endsAt) &&
-          startsAt.minusMinutes(SCHEDULE_DAILY_OVERLAP_MINS).isBefore(si2.endsAt)
-        else
-          (
-            s1.variant.exotic ||  // overlapping exotic variant
-              s1.hasMaxRating ||  // overlapping same rating limit
-              s1.similarSpeed(s2) // overlapping similar
-          ) && s1.similarConditions(s2) && overlaps(si2)
-      )
-
-    /** Kept in sync with [[conflictsWithFailOnUsurp]].
-      */
-    def conflictsWith(scheds: Iterable[ScheduleWithInterval]): Boolean =
-      scheds.exists(conflictsWith)
-
-    /** Kept in sync with [[conflictsWith]].
-      *
-      * Raises an exception if a tourney is incorrectly usurped.
-      */
-    @throws[IllegalStateException]("if a tourney is incorrectly usurped")
-    def conflictsWithFailOnUsurp(scheds: Iterable[ScheduleWithInterval]) =
-      val conflicts   = scheds.filter(conflictsWith).toSeq
-      val okConflicts = conflicts.filter(_.schedule.freq >= schedule.freq)
-      if conflicts.nonEmpty && okConflicts.isEmpty then
-        throw new IllegalStateException(s"Schedule [$schedule] usurped by ${conflicts}")
-      conflicts.nonEmpty
-
-  case class Plan(schedule: Schedule, buildFunc: Option[Tournament => Tournament])
-      extends ScheduleWithInterval:
+  case class Plan(schedule: Schedule, startsAt: Instant, buildFunc: Option[Tournament => Tournament])
+      extends PlanBuilder.ScheduleWithInterval:
 
     def build(using Translate): Tournament =
-      val t = Tournament.scheduleAs(withConditions(schedule), minutes)
+      val t = Tournament.scheduleAs(withConditions(schedule), startsAt, minutes)
       buildFunc.fold(t) { _(t) }
 
     def map(f: Tournament => Tournament) = copy(
       buildFunc = buildFunc.fold(f)(f.compose).some
     )
 
-    override def startsAt = schedule.atInstant
-
     def minutes = durationFor(schedule)
 
     override def duration = java.time.Duration.ofMinutes(minutes)
@@ -423,21 +367,3 @@ object Schedule:
           accountAge = none,
           allowList = none
         )
-
-  /** Given a list of existing schedules and a list of possible new plans, returns a subset of the possible
-    * plans that do not conflict with either the existing schedules or with themselves. Intended to produce
-    * identical output to [[SchedulerTestHelpers.ExperimentalPruner.pruneConflictsFailOnUsurp]], but this
-    * variant is more readable and has lower potential for bugs.
-    */
-  private[tournament] def pruneConflicts[A <: ScheduleWithInterval](
-      existingSchedules: Iterable[ScheduleWithInterval],
-      possibleNewPlans: Iterable[A]
-  ): List[A] =
-    var allPlannedSchedules = existingSchedules.toList
-    possibleNewPlans
-      .foldLeft(Nil): (newPlans, p) =>
-        if p.conflictsWith(allPlannedSchedules) then newPlans
-        else
-          allPlannedSchedules ::= p
-          p :: newPlans
-      .reverse
diff --git a/modules/tournament/src/main/Tournament.scala b/modules/tournament/src/main/Tournament.scala
index 16ed6895a4d9..8aedd6fe67cf 100644
--- a/modules/tournament/src/main/Tournament.scala
+++ b/modules/tournament/src/main/Tournament.scala
@@ -155,7 +155,6 @@ case class Tournament(
 case class EnterableTournaments(tours: List[Tournament], scheduled: List[Tournament])
 
 object Tournament:
-
   val minPlayers = 2
 
   def fromSetup(setup: TournamentSetup)(using me: Me) =
@@ -186,7 +185,7 @@ object Tournament:
       hasChat = setup.hasChat | true
     )
 
-  def scheduleAs(sched: Schedule, minutes: Int)(using Translate) =
+  def scheduleAs(sched: Schedule, startsAt: Instant, minutes: Int)(using Translate) =
     Tournament(
       id = makeId,
       name = sched.name(full = false),
@@ -201,7 +200,7 @@ object Tournament:
       mode = Mode.Rated,
       conditions = sched.conditions,
       schedule = sched.some,
-      startsAt = sched.atInstant.plusSeconds(ThreadLocalRandom.nextInt(60))
+      startsAt = startsAt
     )
 
   def tournamentUrl(tourId: TourId): String = s"https://lichess.org/tournament/$tourId"
diff --git a/modules/tournament/src/main/TournamentScheduler.scala b/modules/tournament/src/main/TournamentScheduler.scala
index a9e9e28968b8..d2bc6f71b768 100644
--- a/modules/tournament/src/main/TournamentScheduler.scala
+++ b/modules/tournament/src/main/TournamentScheduler.scala
@@ -11,27 +11,6 @@ import lila.common.LilaScheduler
 import lila.core.i18n.Translator
 import lila.gathering.Condition
 
-/** This case class (and underlying trait) exists to ensure conflicts are checked against a tournament's true
-  * interval, rather than the interval which could be inferred from the tournament's schedule parameter via
-  * [[Schedule.durationFor]]
-  *
-  * Such a mismatch could occur if durationFor is modified and existing tournaments are rehydrated from db.
-  * Another source of mismatch is that tourney actual start is tweaked from scheduled start by a random number
-  * of seconds (see [[Tournament.scheduleAs]])
-  */
-private[tournament] case class ConcreteSchedule(
-    schedule: Schedule,
-    startsAt: Instant,
-    duration: java.time.Duration
-) extends Schedule.ScheduleWithInterval
-
-private[tournament] case class ConcreteTourney(
-    tournament: Tournament,
-    schedule: Schedule,
-    startsAt: Instant,
-    duration: java.time.Duration
-) extends Schedule.ScheduleWithInterval
-
 final private class TournamentScheduler(tournamentRepo: TournamentRepo)(using
     Executor,
     Scheduler,
@@ -43,18 +22,10 @@ final private class TournamentScheduler(tournamentRepo: TournamentRepo)(using
     tournamentRepo.scheduledUnfinished.flatMap: dbScheds =>
       try
         val plans = TournamentScheduler.allWithConflicts()
-        val possibleTourneys = plans.map(p => (p.schedule, p.build)).map { case (s, t) =>
-          ConcreteTourney(t, s, t.startsAt, t.duration)
-        }
-
-        val existingSchedules = dbScheds.flatMap { t =>
-          // Ignore tournaments with schedule=None - they never conflict.
-          t.schedule.map { ConcreteSchedule(_, t.startsAt, t.duration) }
-        }
 
-        val prunedTourneys = Schedule.pruneConflicts(existingSchedules, possibleTourneys)
+        val filteredPlans = PlanBuilder.filterAndStaggerPlans(dbScheds, plans)
 
-        tournamentRepo.insert(prunedTourneys.map(_.tournament)).logFailure(logger)
+        tournamentRepo.insert(filteredPlans.map(_.build)).logFailure(logger)
       catch
         case e: Exception =>
           logger.error(s"failed to schedule all: ${e.getMessage}")
@@ -492,7 +463,7 @@ Thank you all, you rock!""".some,
               ).plan
             )
       }
-    ).flatten.filter(_.schedule.atInstant.isAfter(rightNow))
+    ).flatten.filter(_.startsAt.isAfter(rightNow))
 
   private def atTopOfHour(rightNow: Instant, hourDelta: Int): LocalDateTime =
     rightNow.plusHours(hourDelta).dateTime.withMinute(0)
diff --git a/modules/tournament/src/test/PlanBuilderTest.scala b/modules/tournament/src/test/PlanBuilderTest.scala
new file mode 100644
index 000000000000..eda05042d287
--- /dev/null
+++ b/modules/tournament/src/test/PlanBuilderTest.scala
@@ -0,0 +1,65 @@
+package lila.tournament
+
+class PlanBuilderTest extends munit.FunSuite:
+
+  test("findMaxSpaceSlot"):
+    def assertSlotFull(low: Float, hi: Float, existing: Seq[Float], expected: Float) =
+      assertEquals(
+        PlanBuilder.findMinimalGoodSlot(low, hi, existing),
+        expected,
+        s"low=$low hi=$hi existing=$existing"
+      )
+
+    def assertSlot(existing: Seq[Float], expected: Float) =
+      assertSlotFull(0f, 10f, existing, expected)
+
+    // Edge case: no existing slots (use low)
+    assertSlot(Nil, 0f)
+
+    // Lowest maximal gap slot is returned
+    assertSlot(Seq(5f), 0f)
+    assertSlot(Seq(4f, 6f), 0f)
+
+    // Middle is prioritized over high when equiv
+    assertSlot(Seq(1f, 7f), 4f)
+
+    // Middle is used if high and low are worse
+    assertSlot(Seq(2f, 8f), 5f)
+
+    // Finds slot correctly.
+    assertSlot(Seq(0f), 10f)
+    assertSlot(Seq(10f), 0f)
+    assertSlot(Seq(4f), 10f)
+    assertSlot(Seq(0f, 10f), 5f)
+    assertSlot(Seq(0f, 5f, 10f), 2.5f)
+    assertSlot(Seq(0f, 2.5f, 5f, 10f), 7.5f)
+    assertSlot(Seq(0f, 2.5f, 5f, 7.5f, 10f), 1.25f)
+    assertSlot(Seq(0f, 2.5f, 7.5f), 5f)
+
+    // Edge case: low == hi
+    assertSlotFull(0f, 0f, Nil, 0f)
+    assertSlotFull(0f, 0f, Seq(0f), 0f)
+
+    // Edge case: unrepresentable floats
+    assertSlotFull(0f, .1f, Nil, 0f)
+    assertSlotFull(0f, .1f, Seq(0f), .1f)
+    assertSlotFull(.1f, .2f, Seq(.1f, .2f), .15f)
+
+    // Unlikely edge case: negatives
+    assertSlotFull(-10f, -5f, Nil, -10f)
+    assertSlotFull(-10f, -5f, Seq(-10f), -5f)
+    assertSlotFull(-10f, -5f, Seq(-5f), -10f)
+    assertSlotFull(-10f, -5f, Seq(-10f, -5f), -7.5f)
+
+    // Extremely unlikely edge case: float extremes
+    import Float.{ MinValue as MinF, MaxValue as MaxF }
+    assertSlotFull(MinF, 0f, Nil, MinF)
+    assertSlotFull(MinF, 0f, Seq(MinF), 0f)
+    assertSlotFull(MinF, 0f, Seq(0f), MinF)
+    assertSlotFull(0f, MaxF, Nil, 0f)
+    assertSlotFull(0f, MaxF, Seq(0f), MaxF)
+    assertSlotFull(MinF, MaxF, Nil, MinF)
+    assertSlotFull(MinF, MaxF, Seq(MinF), MaxF)
+
+    // Extremely unlikely edge case: extreme ranges
+    assertSlotFull(MinF, MaxF, Seq(MinF, MaxF), 0f)
diff --git a/modules/tournament/src/test/ScheduleTestHelpers.scala b/modules/tournament/src/test/ScheduleTestHelpers.scala
index d4c65cce54e7..aaabae443844 100644
--- a/modules/tournament/src/test/ScheduleTestHelpers.scala
+++ b/modules/tournament/src/test/ScheduleTestHelpers.scala
@@ -1,5 +1,7 @@
 package lila.tournament
 
+import PlanBuilder.{ ScheduleWithInterval, SCHEDULE_DAILY_OVERLAP_MINS }
+
 object ScheduleTestHelpers:
   def planSortKey(p: Schedule.Plan) =
     val s = p.schedule
@@ -88,7 +90,7 @@ object ScheduleTestHelpers:
       existingSchedules.foreach { s => getAllHours(s).foreach { addToMap(_, s) } }
 
       possibleNewPlans
-        .foldLeft(List[A]()): (newPlans, p) =>
+        .foldLeft(Nil): (newPlans, p) =>
           val potentialConflicts = getConflictingHours(p).flatMap { hourMap.getOrElse(_, Nil) }
           if p.conflictsWithFailOnUsurp(potentialConflicts) then newPlans
           else
diff --git a/modules/tournament/src/test/SchedulerTest.scala b/modules/tournament/src/test/SchedulerTest.scala
index 7a6f4826a17e..3d97edc062ca 100644
--- a/modules/tournament/src/test/SchedulerTest.scala
+++ b/modules/tournament/src/test/SchedulerTest.scala
@@ -79,7 +79,7 @@ class SchedulerTest extends munit.FunSuite:
     )
 
   test("pruneConflict methods produce identical results"):
-    val prescheduled = Schedule.pruneConflicts(
+    val prescheduled = PlanBuilder.pruneConflicts(
       List.empty,
       TournamentScheduler.allWithConflicts(instantOf(2024, 7, 31, 23, 0))
     )
@@ -89,7 +89,7 @@ class SchedulerTest extends munit.FunSuite:
     }
     assertEquals(
       ExperimentalPruner.pruneConflictsFailOnUsurp(prescheduled, allTourneys),
-      Schedule.pruneConflicts(prescheduled, allTourneys)
+      PlanBuilder.pruneConflicts(prescheduled, allTourneys)
     )
 
   test("2024-09-05 - thursday, summer"):

From d4c163a073fe669151660bba71418afd2f8b135d Mon Sep 17 00:00:00 2001
From: Isaac Levy <isaac.r.levy@gmail.com>
Date: Tue, 24 Sep 2024 12:16:05 -0400
Subject: [PATCH 2/5] Specialize method to use Longs

---
 modules/tournament/src/main/PlanBuilder.scala | 41 +++++-------
 .../tournament/src/test/PlanBuilderTest.scala | 65 +++++++------------
 2 files changed, 41 insertions(+), 65 deletions(-)

diff --git a/modules/tournament/src/main/PlanBuilder.scala b/modules/tournament/src/main/PlanBuilder.scala
index 317f0be80cc6..4285cc5d358c 100644
--- a/modules/tournament/src/main/PlanBuilder.scala
+++ b/modules/tournament/src/main/PlanBuilder.scala
@@ -126,11 +126,11 @@ object PlanBuilder:
       .iteratorFrom(originalStart)
       .takeWhile(_ <= maxConflictAt)
       // Shift events times to be relative to original plan, to avoid loss of precision
-      .map(s => (s.toEpochMilli - originalStartMs).toFloat)
+      .map(_.toEpochMilli - originalStartMs)
       .toSeq
 
-    val staggerMs = findMinimalGoodSlot(0f, maxStaggerMs, offsetsWithSimilarStart)
-    originalStart.plusMillis(staggerMs.toLong)
+    val staggerMs = findMinimalGoodSlot(0L, maxStaggerMs, offsetsWithSimilarStart)
+    originalStart.plusMillis(staggerMs)
 
   /** Given existing tourneys and possible new plans, returns new Plan objects that are staggered to avoid
     * starting at the exact same time as other plans or tourneys. Does NOT filter for conflicts.
@@ -153,12 +153,12 @@ object PlanBuilder:
     * because that means tourneys start sooner, but we also want tournaments to be spaced out to avoid server
     * DDOS.
     *
-    * The method uses Floats. Assuming the original plans use whole numbers (of seconds), successive staggers
-    * will be whole multiples of a negative power of 2, and so are be exactly representable as a Float. Neat!
+    * The method uses Longs for convenience based on usage, but it could easily be specialized to use floating
+    * points or other representations.
     *
-    * Behavior is only loosely defined when the length of [low, hi] approaches or exceeds [[Float.MaxValue]]
-    * or when the range is centered around a large number and loses precision. Neither of these scenarios is
-    * how the function is expected to be used in practice.
+    * Overflows are *not* checked, because although this method uses Longs, its arguments are expected to be
+    * small (e.g. smaller than [[Short.MaxValue]]), and so internal math is not expected to come anywhere near
+    * a Long overflow.
     *
     * @param hi
     *   must be >= low
@@ -168,27 +168,20 @@ object PlanBuilder:
     * @return
     *   the lowest value in [low, hi] with maximal distance to elts of sortedExisting
     */
-  private[tournament] def findMinimalGoodSlot(low: Float, hi: Float, sortedExisting: Iterable[Float]): Float =
-    // Computations use doubles to avoid loss of precision and rounding errors.
+  private[tournament] def findMinimalGoodSlot(low: Long, hi: Long, sortedExisting: Iterable[Long]): Long =
     if sortedExisting.isEmpty then low
     else
       val iter    = sortedExisting.iterator
-      var prevElt = iter.next.toDouble
-      // Fake gap low to check later (i.e. maxGapLow < low)
-      var maxGapLow = Double.NegativeInfinity
-      // nothing is at low element so gap is equiv to 2x
-      var maxGapLen = (prevElt - low) * 2.0
+      var prevElt = iter.next
+      // nothing is at low element so gap is equiv to 2x size, centered at low
+      var maxGapLow = low - (prevElt - low)
+      var maxGapLen = (prevElt - low) * 2L
       while iter.hasNext do
-        val elt = iter.next.toDouble
+        val elt = iter.next
         if elt - prevElt > maxGapLen then
           maxGapLow = prevElt
           maxGapLen = elt - prevElt
         prevElt = elt
-      // Use hi if it's strictly better than all other gaps. Since nothing is at hi, gap is equiv
-      // to 2x maxGapLen.
-      if (hi - prevElt) * 2.0 > maxGapLen then hi
-      // Else, use the first best slot, whose first candidate is low. Using a special case for low
-      // guarantees we always return in the interval [low, high], and don't have to be quite as
-      // vigilant with floating point rounding errors.
-      else if maxGapLow < low then low
-      else (maxGapLow + maxGapLen * 0.5).toFloat
+      // Use hi only if it's strictly better than all other gaps.
+      if (hi - prevElt) * 2L > maxGapLen then hi
+      else maxGapLow + maxGapLen / 2L
diff --git a/modules/tournament/src/test/PlanBuilderTest.scala b/modules/tournament/src/test/PlanBuilderTest.scala
index eda05042d287..9bebf110bb33 100644
--- a/modules/tournament/src/test/PlanBuilderTest.scala
+++ b/modules/tournament/src/test/PlanBuilderTest.scala
@@ -3,63 +3,46 @@ package lila.tournament
 class PlanBuilderTest extends munit.FunSuite:
 
   test("findMaxSpaceSlot"):
-    def assertSlotFull(low: Float, hi: Float, existing: Seq[Float], expected: Float) =
+    def assertSlotFull(low: Long, hi: Long, existing: Seq[Long], expected: Long) =
       assertEquals(
         PlanBuilder.findMinimalGoodSlot(low, hi, existing),
         expected,
         s"low=$low hi=$hi existing=$existing"
       )
 
-    def assertSlot(existing: Seq[Float], expected: Float) =
-      assertSlotFull(0f, 10f, existing, expected)
+    def assertSlot(existing: Seq[Long], expected: Long) =
+      assertSlotFull(0L, 100L, existing, expected)
 
     // Edge case: no existing slots (use low)
-    assertSlot(Nil, 0f)
+    assertSlot(Nil, 0L)
 
-    // Lowest maximal gap slot is returned
-    assertSlot(Seq(5f), 0f)
-    assertSlot(Seq(4f, 6f), 0f)
+    // lowest maximal gap slot is returned
+    assertSlot(Seq(50L), 0L)
+    assertSlot(Seq(40L, 60L), 0L)
 
     // Middle is prioritized over high when equiv
-    assertSlot(Seq(1f, 7f), 4f)
+    assertSlot(Seq(10L, 70L), 40L)
 
     // Middle is used if high and low are worse
-    assertSlot(Seq(2f, 8f), 5f)
+    assertSlot(Seq(20L, 80L), 50L)
 
     // Finds slot correctly.
-    assertSlot(Seq(0f), 10f)
-    assertSlot(Seq(10f), 0f)
-    assertSlot(Seq(4f), 10f)
-    assertSlot(Seq(0f, 10f), 5f)
-    assertSlot(Seq(0f, 5f, 10f), 2.5f)
-    assertSlot(Seq(0f, 2.5f, 5f, 10f), 7.5f)
-    assertSlot(Seq(0f, 2.5f, 5f, 7.5f, 10f), 1.25f)
-    assertSlot(Seq(0f, 2.5f, 7.5f), 5f)
+    assertSlot(Seq(0L), 100L)
+    assertSlot(Seq(100L), 0L)
+    assertSlot(Seq(40L), 100L)
+    assertSlot(Seq(0L, 100L), 50L)
+    assertSlot(Seq(0L, 50L, 100L), 25L)
+    assertSlot(Seq(0L, 25L, 50L, 100L), 75L)
+    assertSlot(Seq(0L, 25L, 50L, 75L, 100L), 12L) // Rounds down
+    assertSlot(Seq(0L, 25L, 75L), 50L)
 
     // Edge case: low == hi
-    assertSlotFull(0f, 0f, Nil, 0f)
-    assertSlotFull(0f, 0f, Seq(0f), 0f)
-
-    // Edge case: unrepresentable floats
-    assertSlotFull(0f, .1f, Nil, 0f)
-    assertSlotFull(0f, .1f, Seq(0f), .1f)
-    assertSlotFull(.1f, .2f, Seq(.1f, .2f), .15f)
+    assertSlotFull(0L, 0L, Nil, 0L)
+    assertSlotFull(0L, 0L, Seq(0L), 0L)
 
     // Unlikely edge case: negatives
-    assertSlotFull(-10f, -5f, Nil, -10f)
-    assertSlotFull(-10f, -5f, Seq(-10f), -5f)
-    assertSlotFull(-10f, -5f, Seq(-5f), -10f)
-    assertSlotFull(-10f, -5f, Seq(-10f, -5f), -7.5f)
-
-    // Extremely unlikely edge case: float extremes
-    import Float.{ MinValue as MinF, MaxValue as MaxF }
-    assertSlotFull(MinF, 0f, Nil, MinF)
-    assertSlotFull(MinF, 0f, Seq(MinF), 0f)
-    assertSlotFull(MinF, 0f, Seq(0f), MinF)
-    assertSlotFull(0f, MaxF, Nil, 0f)
-    assertSlotFull(0f, MaxF, Seq(0f), MaxF)
-    assertSlotFull(MinF, MaxF, Nil, MinF)
-    assertSlotFull(MinF, MaxF, Seq(MinF), MaxF)
-
-    // Extremely unlikely edge case: extreme ranges
-    assertSlotFull(MinF, MaxF, Seq(MinF, MaxF), 0f)
+    assertSlotFull(-10L, -5L, Nil, -10L)
+    assertSlotFull(-10L, -5L, Seq(-10L), -5L)
+    assertSlotFull(-10L, -5L, Seq(-5L), -10L)
+    assertSlotFull(-10L, -5L, Seq(-10L, -5L), -8L) // Rounds down when negative
+    assertSlotFull(-1L, 2L, Seq(-1L, 2L), 0L)      // Rounds down when positive

From d9254df4cc3a6dd1fce467e546f8b683dfbe486c Mon Sep 17 00:00:00 2001
From: Isaac Levy <isaac.r.levy@gmail.com>
Date: Tue, 24 Sep 2024 12:44:49 -0400
Subject: [PATCH 3/5] Misc cleanup

---
 modules/tournament/src/main/PlanBuilder.scala | 29 ++++++++-----------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/modules/tournament/src/main/PlanBuilder.scala b/modules/tournament/src/main/PlanBuilder.scala
index 4285cc5d358c..95e4b0ec9f12 100644
--- a/modules/tournament/src/main/PlanBuilder.scala
+++ b/modules/tournament/src/main/PlanBuilder.scala
@@ -2,8 +2,6 @@ package lila.tournament
 
 import scala.collection.mutable
 import scala.collection.SortedSet
-// For comparing Instants
-import scala.math.Ordering.Implicits.infixOrderingOps
 
 import Schedule.Plan
 
@@ -15,9 +13,8 @@ object PlanBuilder:
     */
   private[tournament] val SCHEDULE_DAILY_OVERLAP_MINS = 690 // 11.5 hours
 
-  // 2/3 of a min ensures good spacing from tourneys starting the next minute, which aren't explicitly
-  // considered when calculating ideal stagger. It also keeps at least or better spacing than the prior
-  // random [0, 60) sec stagger.
+  // 40s ensures good spacing from tourneys starting the next minute, which aren't considered when
+  // calculating ideal stagger. It also keeps better spacing than the prior random [0, 60) sec stagger.
   private[tournament] val MAX_STAGGER_MS = 40_000
 
   private[tournament] abstract class ScheduleWithInterval:
@@ -86,6 +83,7 @@ object PlanBuilder:
     val prunedPlans = pruneConflicts(existingWithScheduledStart, plans)
 
     // Unlike pruning, stagger plans even against Tourneys with schedule=None.
+    // This probably doesn't affect anything, but seems prudent.
     plansWithStagger(existingTourneys, prunedPlans)
 
   /** Given a list of existing schedules and a list of possible new plans, returns a subset of the possible
@@ -106,16 +104,14 @@ object PlanBuilder:
           p :: newPlans
       .reverse
 
-  /** Given a plan, return an adjusted start time that minimizes conflicts with existing events.
-    *
-    * This method assumes that no event starts immediately before plan or after the max stagger, in terms of
-    * impacting server performance by concurrent events.
+  /** Given a plan, return an adjusted Plan with a start time that minimizes conflicts with existing events.
     */
-  private[tournament] def getAdjustedStart(
+  private[tournament] def staggerPlan(
       plan: Plan,
       existingEvents: SortedSet[Instant],
       maxStaggerMs: Long
-  ): Instant =
+  ): Plan =
+    import scala.math.Ordering.Implicits.infixOrderingOps // For comparing Instants.
 
     val originalStart   = plan.startsAt
     val originalStartMs = originalStart.toEpochMilli
@@ -125,12 +121,11 @@ object PlanBuilder:
     val offsetsWithSimilarStart = existingEvents
       .iteratorFrom(originalStart)
       .takeWhile(_ <= maxConflictAt)
-      // Shift events times to be relative to original plan, to avoid loss of precision
       .map(_.toEpochMilli - originalStartMs)
       .toSeq
 
     val staggerMs = findMinimalGoodSlot(0L, maxStaggerMs, offsetsWithSimilarStart)
-    originalStart.plusMillis(staggerMs)
+    plan.copy(startsAt = originalStart.plusMillis(staggerMs))
 
   /** Given existing tourneys and possible new plans, returns new Plan objects that are staggered to avoid
     * starting at the exact same time as other plans or tourneys. Does NOT filter for conflicts.
@@ -140,13 +135,13 @@ object PlanBuilder:
       plans: Iterable[Plan]
   ): List[Plan] =
     // Determine stagger using the actual (staggered) start time of existing and new tourneys.
-    val allTourneyStarts = mutable.TreeSet.from(existingTourneys.map(_.startsAt))
+    val allTourneyInstants = mutable.TreeSet.from(existingTourneys.map(_.startsAt))
 
     plans
       .foldLeft(Nil): (allAdjustedPlans, plan) =>
-        val adjustedStart = getAdjustedStart(plan, allTourneyStarts, MAX_STAGGER_MS)
-        allTourneyStarts += adjustedStart
-        plan.copy(startsAt = adjustedStart) :: allAdjustedPlans
+        val adjustedPlan = staggerPlan(plan, allTourneyInstants, MAX_STAGGER_MS)
+        allTourneyInstants += adjustedPlan.startsAt
+        adjustedPlan :: allAdjustedPlans
       .reverse
 
   /** This method is used find a good stagger value for a new tournament. We want stagger as low as possible,

From a02b1e68801bb1a296e5a6a36d8355805259bd51 Mon Sep 17 00:00:00 2001
From: Isaac Levy <isaac.r.levy@gmail.com>
Date: Tue, 24 Sep 2024 12:52:29 -0400
Subject: [PATCH 4/5] More cleanup

---
 modules/tournament/src/main/PlanBuilder.scala | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/modules/tournament/src/main/PlanBuilder.scala b/modules/tournament/src/main/PlanBuilder.scala
index 95e4b0ec9f12..df14c4a55987 100644
--- a/modules/tournament/src/main/PlanBuilder.scala
+++ b/modules/tournament/src/main/PlanBuilder.scala
@@ -82,9 +82,9 @@ object PlanBuilder:
 
     val prunedPlans = pruneConflicts(existingWithScheduledStart, plans)
 
+    // Determine stagger using the actual (staggered) start time of existing tourneys.
     // Unlike pruning, stagger plans even against Tourneys with schedule=None.
-    // This probably doesn't affect anything, but seems prudent.
-    plansWithStagger(existingTourneys, prunedPlans)
+    plansWithStagger(existingTourneys.map(_.startsAt), prunedPlans)
 
   /** Given a list of existing schedules and a list of possible new plans, returns a subset of the possible
     * plans that do not conflict with either the existing schedules or with themselves. Intended to produce
@@ -131,16 +131,15 @@ object PlanBuilder:
     * starting at the exact same time as other plans or tourneys. Does NOT filter for conflicts.
     */
   private[tournament] def plansWithStagger(
-      existingTourneys: Iterable[Tournament],
+      existingEvents: Iterable[Instant],
       plans: Iterable[Plan]
   ): List[Plan] =
-    // Determine stagger using the actual (staggered) start time of existing and new tourneys.
-    val allTourneyInstants = mutable.TreeSet.from(existingTourneys.map(_.startsAt))
+    val allInstants = mutable.TreeSet.from(existingEvents)
 
     plans
       .foldLeft(Nil): (allAdjustedPlans, plan) =>
-        val adjustedPlan = staggerPlan(plan, allTourneyInstants, MAX_STAGGER_MS)
-        allTourneyInstants += adjustedPlan.startsAt
+        val adjustedPlan = staggerPlan(plan, allInstants, MAX_STAGGER_MS)
+        allInstants += adjustedPlan.startsAt
         adjustedPlan :: allAdjustedPlans
       .reverse
 

From 225fb66607c78f71739fd24b3af33eaba04a05f8 Mon Sep 17 00:00:00 2001
From: Isaac Levy <isaac.r.levy@gmail.com>
Date: Tue, 24 Sep 2024 13:19:51 -0400
Subject: [PATCH 5/5] More cleanup2

---
 modules/tournament/src/main/PlanBuilder.scala | 20 ++++++++++++-------
 .../src/main/TournamentScheduler.scala        |  7 ++-----
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/modules/tournament/src/main/PlanBuilder.scala b/modules/tournament/src/main/PlanBuilder.scala
index df14c4a55987..53d2b42868df 100644
--- a/modules/tournament/src/main/PlanBuilder.scala
+++ b/modules/tournament/src/main/PlanBuilder.scala
@@ -3,7 +3,8 @@ package lila.tournament
 import scala.collection.mutable
 import scala.collection.SortedSet
 
-import Schedule.Plan
+import lila.core.i18n.Translate
+import lila.tournament.Schedule.Plan
 
 object PlanBuilder:
   /** Max window for daily or better schedules to be considered overlapping (i.e. if one starts within X hrs
@@ -70,10 +71,12 @@ object PlanBuilder:
       val duration: java.time.Duration
   ) extends ScheduleWithInterval
 
-  private[tournament] def filterAndStaggerPlans(
+  private[tournament] def getNewTourneys(
       existingTourneys: Iterable[Tournament],
-      plans: Iterable[Plan]
-  ): List[Plan] =
+      now: Instant = nowInstant
+  )(using Translate): List[Tournament] =
+    val plans = TournamentScheduler.allWithConflicts()
+
     // Prune plans using the unstaggered scheduled start time.
     val existingWithScheduledStart = existingTourneys.flatMap { t =>
       // Ignore tournaments with schedule=None - they never conflict.
@@ -82,9 +85,12 @@ object PlanBuilder:
 
     val prunedPlans = pruneConflicts(existingWithScheduledStart, plans)
 
-    // Determine stagger using the actual (staggered) start time of existing tourneys.
-    // Unlike pruning, stagger plans even against Tourneys with schedule=None.
-    plansWithStagger(existingTourneys.map(_.startsAt), prunedPlans)
+    plansWithStagger(
+      // Determine new staggers based on actual (staggered) start time of existing tourneys.
+      // Unlike pruning, stagger considers Tourneys with schedule=None.
+      existingTourneys.map(_.startsAt),
+      prunedPlans
+    ).map(_.build) // Build Tournament objects from plans
 
   /** Given a list of existing schedules and a list of possible new plans, returns a subset of the possible
     * plans that do not conflict with either the existing schedules or with themselves. Intended to produce
diff --git a/modules/tournament/src/main/TournamentScheduler.scala b/modules/tournament/src/main/TournamentScheduler.scala
index d2bc6f71b768..3cb27d4f8ea7 100644
--- a/modules/tournament/src/main/TournamentScheduler.scala
+++ b/modules/tournament/src/main/TournamentScheduler.scala
@@ -21,11 +21,8 @@ final private class TournamentScheduler(tournamentRepo: TournamentRepo)(using
     given play.api.i18n.Lang = lila.core.i18n.defaultLang
     tournamentRepo.scheduledUnfinished.flatMap: dbScheds =>
       try
-        val plans = TournamentScheduler.allWithConflicts()
-
-        val filteredPlans = PlanBuilder.filterAndStaggerPlans(dbScheds, plans)
-
-        tournamentRepo.insert(filteredPlans.map(_.build)).logFailure(logger)
+        val tourneysToAdd = PlanBuilder.getNewTourneys(dbScheds)
+        tournamentRepo.insert(tourneysToAdd).logFailure(logger)
       catch
         case e: Exception =>
           logger.error(s"failed to schedule all: ${e.getMessage}")