From f845b33d0ed70413b9652d1cb88b29e7db05920d Mon Sep 17 00:00:00 2001 From: razam-sherwani Date: Tue, 18 Aug 2026 20:51:47 -0400 Subject: [PATCH] Stop the stale rescheduler cloning live and undeliverable matches (#174) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findStaleWaitingMatches selected status IN ('waiting','in_progress') and the rescheduler requeued each without cancelling the original, so a wedged match was re-enqueued every minute past the threshold and times_scheduled climbed — load multiplied under the exact stress the sweep fires in. - Sweep only 'waiting'. A match becomes 'in_progress' only once a worker emits a "started" update (GameMatchResultHandler), so requeuing one clones live work; a wedged in_progress match is the per-match watchdog's job (ByteFight-GT/cs3600_2026#35), not this sweep's. - Fail a match instead of requeuing it once it has been scheduled gamematch.max-reschedules times (default 5), via the existing finalizeMatchResult + failed status, so an undeliverable waiting match can't loop forever. - Honour the existing gamematch.requeue-stale flag, which was defined and set true but read nowhere — it's now a real kill-switch. Co-Authored-By: Claude Opus 5 --- .../StaleGameMatchRescheduler.java | 38 ++++- .../gamematch/infra/GameMatchProperties.java | 7 + .../gamematch/infra/GameMatchRepository.java | 2 +- src/main/resources/application.yml | 1 + .../StaleGameMatchReschedulerIT.java | 141 ++++++++++++++++++ 5 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 src/test/java/org/bytefight/webserver/gamematch/StaleGameMatchReschedulerIT.java diff --git a/src/main/java/org/bytefight/webserver/gamematch/application/StaleGameMatchRescheduler.java b/src/main/java/org/bytefight/webserver/gamematch/application/StaleGameMatchRescheduler.java index 622a401a..b2252592 100644 --- a/src/main/java/org/bytefight/webserver/gamematch/application/StaleGameMatchRescheduler.java +++ b/src/main/java/org/bytefight/webserver/gamematch/application/StaleGameMatchRescheduler.java @@ -8,6 +8,7 @@ import java.util.List; import org.bytefight.webserver.gamematch.domain.GameMatch; +import org.bytefight.webserver.gamematch.domain.MatchStatus; import org.bytefight.webserver.gamematch.infra.GameMatchProperties; import org.bytefight.webserver.gamematch.infra.GameMatchRepository; import org.springframework.scheduling.annotation.Scheduled; @@ -25,16 +26,47 @@ public class StaleGameMatchRescheduler { @Scheduled(fixedRate = 60000) @Transactional public void rescheduleStaleGameMatches() { + if (!matchProperties.isRequeueStale()) { + return; + } + + // Only 'waiting' matches are swept: a match becomes 'in_progress' only once a worker has + // emitted a "started" update, so requeuing one would clone live work. A wedged in_progress + // match is the per-match watchdog's responsibility (cs3600_2026#35), not this sweep's. List staleGameMatches = gameMatchRepository.findStaleWaitingMatches( Instant.now().minus(matchProperties.getStaleThresholdMinutes(), ChronoUnit.MINUTES)); - if (!staleGameMatches.isEmpty()) { - log.info("Rescheduling {} stale game matches", staleGameMatches.size()); + if (staleGameMatches.isEmpty()) { + return; } + int rescheduled = 0; + int failed = 0; for (GameMatch gameMatch : staleGameMatches) { - gameMatchService.scheduleMatch(gameMatch); + if (gameMatch.getTimesScheduled() >= matchProperties.getMaxReschedules()) { + // Undeliverable: it has been scheduled the maximum number of times and is still waiting. + // Fail it rather than requeue it forever. Guard on 'waiting' so a concurrent worker update + // that moved it to in_progress/finished wins. + int updated = + gameMatchRepository.finalizeMatchResult( + gameMatch.getUuid(), + MatchStatus.failed, + Instant.now(), + List.of(MatchStatus.waiting)); + if (updated > 0) { + failed++; + log.warn( + "Failing stale match {} after {} schedule attempts", + gameMatch.getUuid(), + gameMatch.getTimesScheduled()); + } + } else { + gameMatchService.scheduleMatch(gameMatch); + rescheduled++; + } } + + log.info("Stale sweep: rescheduled {}, failed {}", rescheduled, failed); } } diff --git a/src/main/java/org/bytefight/webserver/gamematch/infra/GameMatchProperties.java b/src/main/java/org/bytefight/webserver/gamematch/infra/GameMatchProperties.java index 19ebebd4..a9024bcf 100644 --- a/src/main/java/org/bytefight/webserver/gamematch/infra/GameMatchProperties.java +++ b/src/main/java/org/bytefight/webserver/gamematch/infra/GameMatchProperties.java @@ -11,4 +11,11 @@ public class GameMatchProperties { private long staleThresholdMinutes; private boolean requeueStale; + + /** + * Maximum number of times a match may be (re)scheduled before the stale sweep gives up and marks + * it failed instead of requeuing it forever. A match is scheduled once on creation, so this is the + * total attempt count, not the retry count. + */ + private int maxReschedules = 5; } diff --git a/src/main/java/org/bytefight/webserver/gamematch/infra/GameMatchRepository.java b/src/main/java/org/bytefight/webserver/gamematch/infra/GameMatchRepository.java index 2b809839..f3eafd4a 100644 --- a/src/main/java/org/bytefight/webserver/gamematch/infra/GameMatchRepository.java +++ b/src/main/java/org/bytefight/webserver/gamematch/infra/GameMatchRepository.java @@ -92,7 +92,7 @@ Page findByCompetitionAndStatus( """ SELECT gm FROM GameMatch gm - WHERE gm.status IN ('waiting', 'in_progress') + WHERE gm.status = 'waiting' AND gm.scheduledAt <= :cutoff """) List findStaleWaitingMatches(@Param("cutoff") Instant cutoff); diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index c7fcadc5..c3466314 100755 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -25,6 +25,7 @@ springdoc: gamematch: requeue-stale: true stale-threshold-minutes: 120 + max-reschedules: 5 storage: max-decompressed-bytes: 524288000 diff --git a/src/test/java/org/bytefight/webserver/gamematch/StaleGameMatchReschedulerIT.java b/src/test/java/org/bytefight/webserver/gamematch/StaleGameMatchReschedulerIT.java new file mode 100644 index 00000000..5a0e848b --- /dev/null +++ b/src/test/java/org/bytefight/webserver/gamematch/StaleGameMatchReschedulerIT.java @@ -0,0 +1,141 @@ +package org.bytefight.webserver.gamematch; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.UUID; + +import org.bytefight.webserver.FullStackIntegrationTestBase; +import org.bytefight.webserver.TestDataFactory; +import org.bytefight.webserver.competition.domain.Competition; +import org.bytefight.webserver.gamematch.application.GameMatchService; +import org.bytefight.webserver.gamematch.application.StaleGameMatchRescheduler; +import org.bytefight.webserver.gamematch.domain.GameMatch; +import org.bytefight.webserver.gamematch.domain.MatchReason; +import org.bytefight.webserver.gamematch.domain.MatchStatus; +import org.bytefight.webserver.gamematch.infra.GameMatchProperties; +import org.bytefight.webserver.gamematch.infra.GameMatchRepository; +import org.bytefight.webserver.storage.domain.FileRecord; +import org.bytefight.webserver.storage.infra.FileRecordRepository; +import org.bytefight.webserver.submission.domain.Submission; +import org.bytefight.webserver.submission.domain.SubmissionValidity; +import org.bytefight.webserver.submission.infra.SubmissionRepository; +import org.bytefight.webserver.team.domain.Team; +import org.bytefight.webserver.user.domain.User; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +class StaleGameMatchReschedulerIT extends FullStackIntegrationTestBase { + @Autowired private StaleGameMatchRescheduler rescheduler; + @Autowired private GameMatchService gameMatchService; + @Autowired private GameMatchRepository gameMatchRepository; + @Autowired private GameMatchProperties matchProperties; + @Autowired private TestDataFactory testDataFactory; + @Autowired private SubmissionRepository submissionRepository; + @Autowired private FileRecordRepository fileRecordRepository; + + @Test + void inProgressMatchPastThresholdIsNotRequeued() { + GameMatch match = staleMatch(MatchStatus.in_progress, 1); + + rescheduler.rescheduleStaleGameMatches(); + + GameMatch after = reload(match); + assertThat(after.getStatus()).isEqualTo(MatchStatus.in_progress); + assertThat(after.getTimesScheduled()).isEqualTo(1); + } + + @Test + void waitingMatchPastThresholdIsRequeuedOnce() { + GameMatch match = staleMatch(MatchStatus.waiting, 1); + + rescheduler.rescheduleStaleGameMatches(); + + GameMatch after = reload(match); + // scheduleMatch re-enqueues it and bumps the counter; it returns to waiting with a fresh + // scheduledAt (so it will not immediately re-trip the threshold). + assertThat(after.getStatus()).isEqualTo(MatchStatus.waiting); + assertThat(after.getTimesScheduled()).isEqualTo(2); + assertThat(after.getScheduledAt()).isAfter(Instant.now().minus(1, ChronoUnit.MINUTES)); + } + + @Test + void waitingMatchAtMaxReschedulesIsFailedNotRequeued() { + GameMatch match = staleMatch(MatchStatus.waiting, matchProperties.getMaxReschedules()); + + rescheduler.rescheduleStaleGameMatches(); + + GameMatch after = reload(match); + assertThat(after.getStatus()).isEqualTo(MatchStatus.failed); + assertThat(after.getFinishedAt()).isNotNull(); + } + + @Test + void sweepIsSkippedWhenRequeueStaleDisabled() { + boolean original = matchProperties.isRequeueStale(); + matchProperties.setRequeueStale(false); + try { + GameMatch match = staleMatch(MatchStatus.waiting, 1); + + rescheduler.rescheduleStaleGameMatches(); + + GameMatch after = reload(match); + assertThat(after.getStatus()).isEqualTo(MatchStatus.waiting); + assertThat(after.getTimesScheduled()).isEqualTo(1); + } finally { + matchProperties.setRequeueStale(original); + } + } + + private GameMatch staleMatch(MatchStatus status, int timesScheduled) { + Competition competition = testDataFactory.createCompetition(); + String ladder = "ranked"; + testDataFactory.createLadder(competition, ladder); + Team teamA = testDataFactory.createTeam(competition, UUID.randomUUID(), false); + Team teamB = testDataFactory.createTeam(competition, UUID.randomUUID(), false); + User user = testDataFactory.createUser(); + + GameMatch match = + gameMatchService.createMatch( + user, + teamA, + teamB, + createSubmission(teamA), + createSubmission(teamB), + ladder, + MatchReason.matchmaking, + null, + null); + match.setStatus(status); + match.setTimesScheduled(timesScheduled); + // Age it well past the stale threshold so the sweep picks it up. + match.setScheduledAt( + Instant.now().minus(matchProperties.getStaleThresholdMinutes() + 5, ChronoUnit.MINUTES)); + return gameMatchRepository.save(match); + } + + private GameMatch reload(GameMatch match) { + return gameMatchRepository.findById(match.getId()).orElseThrow(); + } + + private Submission createSubmission(Team team) { + FileRecord record = + FileRecord.builder() + .uuid(UUID.randomUUID()) + .filename("bot.zip") + .contentType("application/zip") + .size(1L) + .sha256("deadbeef") + .storagePath("/tmp/bot.zip") + .build(); + fileRecordRepository.save(record); + + Submission submission = new Submission(); + submission.setUuid(UUID.randomUUID()); + submission.setTeam(team); + submission.setFileRecord(record); + submission.setValidity(SubmissionValidity.valid); + return submissionRepository.save(submission); + } +}