Skip to content

fix(leaderboards): move match publication to a durable background queue - #42

Open
tintinhamans wants to merge 2 commits into
GeneralsOnlineDevelopmentTeam:mainfrom
tintinhamans:fix/nonblocking-leaderboard-publication
Open

fix(leaderboards): move match publication to a durable background queue#42
tintinhamans wants to merge 2 commits into
GeneralsOnlineDevelopmentTeam:mainfrom
tintinhamans:fix/nonblocking-leaderboard-publication

Conversation

@tintinhamans

@tintinhamans tintinhamans commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

External leaderboard publication previously ran during lobby teardown on the WebSocket processing path. External HTTP latency and retries could therefore delay processing for connected users.

This change records publication work when a match is finalized and processes it asynchronously through a durable database-backed worker.


  • Finalize match history and schedule external publication in the same database transaction.
  • Persist publication attempts, retry time, last error, and completion time in external_publication.
  • Publish pending matches from a background worker with persisted retry backoff.
  • Stop retrying permanent client errors while retaining retries for transient failures.
  • Keep EF Core contexts scoped to individual database operations; no context is held during external HTTP.
  • Separate publication transport from persistence: the external client only sends the match and returns the response, while the worker owns database and session-cache updates.
  • Save rating updates to the database before updating the local session cache.
  • Treat rating-response processing as best effort so it cannot retry an already successful ingest.
  • Remove Polly because retry state is now handled durably by the worker.

Apply upgrade_20260816_external_leaderboard_publication.sql before deploying the updated service.

The worker currently assumes a single service instance.

@STK0Cervanthes

Copy link
Copy Markdown
Contributor

this would also solve #40 right?

@Skyaero42 Skyaero42 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Removing polly is the correct call here I think.

I'm pretty new to the code base and style, so I'm still struggling a bit. Overall this looks good.

One point of attention: the new code throws at a few points. I'm not entirely sure if the throw is properly caught. If it is not, the timer will not be restarted and cleaning up the lobbies will fail.

{
Console.WriteLine($"[ERROR] DetermineLobbyWinnerIfNotPresent failed: {ex.Message}");
SentrySdk.CaptureException(ex);
throw;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure if you want to throw here.

Is it caught somewhere else?
Does it have to be thrown or can the application continue after it has been caught at this level?

Multiple times in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This prevents the exception from being swallowed. It bubbles up to DeleteLobby, logs to Sentry, and returns false, so the lobby persists and the cleanup will retry.

@tintinhamans
tintinhamans marked this pull request as ready for review August 20, 2026 05:00
@tintinhamans tintinhamans changed the title fix(leaderboards): prevent publication from stalling WebSocket processing fix(leaderboards): move match publication to a durable background queue Aug 20, 2026
@tintinhamans
tintinhamans force-pushed the fix/nonblocking-leaderboard-publication branch from 1348d08 to 5a2aa5a Compare August 20, 2026 10:22
@tintinhamans
tintinhamans marked this pull request as draft August 20, 2026 10:33
@tintinhamans
tintinhamans force-pushed the fix/nonblocking-leaderboard-publication branch from 5a2aa5a to f7e0b1e Compare August 20, 2026 11:04
@tintinhamans
tintinhamans marked this pull request as ready for review August 20, 2026 13:45
postToken = sectionPostToken;
}

private static void GetExternalLeaderboardsGetConfig(out string getUrl, out string getToken)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: name has 'get' twice. Recommend GetExternalLeaderboardsConfig

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I’ve renamed it :)

}

// Persist publication before removing the lobby.
await Database.MatchHistory.FinalizeAndScheduleExternalPublication(db, lobby);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This can throw and is not caught. Is that intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It’s handled by DeleteLobby, which logs the error and leaves the lobby registered so the cleanup timer can retry it.

Signed-off-by: tintinhamans <5984296+tintinhamans@users.noreply.github.com>
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.

3 participants