Add task scores processor#119
Merged
Merged
Conversation
…d acc for userstats
richardscull
commented
May 21, 2026
richardscull
left a comment
Member
Author
There was a problem hiding this comment.
We are almost ready for the merge.
Only need to address the comments + double check the tests 🙏
…globally; Use score value (total score for std, pp for relax and non score mods) to check if the current score is better than the peer
… in game mode for user
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is one of the biggest rewrites after a long time, so it might need some additional explanation.
Reasoning for the change
Score submission is one of the core components of the osu! server, even if not the most important one.
Players client send replay file + score data to us using bancho endpoint of
osu-submit-modular-selector.php. Previously it was a simple controller -> service call, but currently controller also ownsprocessingScoresqueue to stop any duplicates of already processing scores, this is achieved by keying all scores to the score hash.After controller preprocessing, all the score submission was handled in a single
ScoreServiceservice bySubmitScoremethod, and it does, let's say, a lot:flowchart TD %% ========================= %% PHASE 1: VALIDATION %% ========================= subgraph P1[Validation and Preprocessing] A[Start SubmitScore] --> B[Get BeatmapSet] B --> C{BeatmapSet valid?} C -- No --> X1[Reject] C -- Yes --> D[Find Beatmap; Retry HTTP call infinitely if they are failing without 'Not found' error] D --> E{Beatmap exists?} E -- No --> X2[Reject] E -- Yes --> F[Parse score] F --> G[Check client version if enforced] G --> H[Check existing score by hash] H --> I{Duplicate score?} I -- Yes --> X3[Reject] I -- No --> J[Calculate performance; Retry HTTP call infinitely if they are failing without 'Not found' error] J --> K{Performance valid?} K -- No --> X4[Reject] K -- Yes --> L[Assign PP] L --> M[Handle replay upload] M --> N{Replay required but missing?} N -- Yes --> X5[Reject] N -- No --> O[Validate mods] O --> P{Invalid mods?} P -- Yes --> X6[Reject] P -- No --> Q{Unsupported mod combo?} Q -- Yes --> X7[Reject] Q -- No --> R{PP too high?} R -- Yes --> X8[Reject and restrict] R -- No --> S[Validate checksums] S --> T{Score valid?} T -- No --> X9[Reject and restrict] end %% ========================= %% PHASE 2: SUBMISSION %% ========================= subgraph P2[Score Submission] T -- Yes --> U[Load user, stats, grades] U --> V[Begin transaction] V --> V1[Update stats and grades] V --> V2[Insert score] V --> V3[Update best scores] V --> V4[Commit transaction] V4 --> W{Transaction success?} W -- No --> X10[Reject] end %% ========================= %% PHASE 3: POST EFFECTS %% ========================= subgraph P3[Post Effects] W -- Yes --> A1[Increment metrics] A1 --> A2[Broadcast websocket] A2 --> A3{Score scoreable?} A3 -- No --> END1[Return early] A3 -- Yes --> A4[Announce first place if needed] A4 --> A8[Unlock medals and achievements] A8 --> END2[Return response] endEven with the most complicated logics being encapsulated in different services, the whole score submission process is a big monolith that gets too complicated to maintain in the current state.
But actually the biggest problem with this structure is that we must wait for any failing HTTP calls for beatmap/pp calculation to proceed BEFORE we save the score into the database. This is fine for small network hiccups, but infinitely bad on a grand scheme of things.
For clarity, let's now count all the problems with the current implementation:
UpdateSubmissionStatusfor the score before you can callUpdateWithScoreon user stats/grades. This is very easy to miss.)Solution(s)
I'll start with mentioning that at the start I wanted to tackle only the first problem without large code changes. But as I continued forward, the scope only grew.
If I recall correctly, I started with creating a single queue table for all the non processed scores, which would be processed later by the cron job in a simple manner. This was the simplest solution, but it wouldn't really help with anything else. Adding any new logic to the score submission is still relatively impossible. So, in the expected way, I tried to split the score service into smaller services.
Which led us to having around 8 new services being introduces (most of them also could be downgraded to utils, but this is not that important), for which we didn't really have the place to go. While yes, I could leave them in the
Services/ScoreServicesdomain, I really disliked this idea since it only would make things harder in the long term.After all the back and forth, I finally decided to create a new
Sunrise.Processingproject, which will store all similar backgrounds and not state machines like score processing.Implementation
I'll start with the notice of this PR also introducing score deletion/recalculation/restoration, I'll clarify how each process works in the following text.
Now, we have this new project of
Sunrise.Processing, I assume this should be clear that it "processes" some entities, but how exactly? Let me go by steps/domainsScores)Jobfor it.Handlersfolder. For the score submission example isScoreSubmissionHandler.3.1. Handlers work in three steps: Prepare -> Commit -> OnCommitted
3.2. Prepare/OnCommitted steps are completely free to be overridden by the handlers if required.
processorsfor any related entities to the domain and completes them in linear order.4.1. Each handler has their own method implementation for each unique unit of work.
4.2. To persist the changes, the processor's work is saved in a context variable, which is created during the prepare step and used until the end of the pipeline.
OnCommittedmethod if it was implemented.flowchart TD A[Hangfire recurring job: ScoreProcessingJob.ProcessQueue] --> B[Claim pending task batch with lease] B --> C{For each claimed task} C --> D[Resolve keyed handler by task type] D --> E[Handler ExecuteAsync] E --> E1[PrepareAsync] E1 -->|failure| F[Mark task failed / retry backoff] E1 -->|success| E2[CommitAsync] E2 --> P6[ScoreCommitPipeline inside DB transaction] P6 -->|fails| R[Throw -> rollback transaction] P6 -->|ok| E3[Commit transaction] E3 --> E4[OnCommitted] E4 -->|submission override| S[Publish side effects] E4 -->|others default| T[No-op] E3 --> U[Cleanup queue entries] U --> V[Success metrics/logging] R --> F F --> W[Failure metrics/logging]With the following system we now have two points of scaling the system functionality if required.
Handlerif we want to add new abilities to interact with the entity.Processorsif we want to add new entities, which may change during the handlers' work.Note
We also have inline call specifically for the scores to be able run the score processing without submitting the job first (since A. it is faster B. client needs stringified response to the score submission)
I tried to explain to my best abilities the new service; hopefully, the code would be the best guidance on that.
Additional notes
This PR introduces massive changes to the codebase, and I'm not planning to actually stop.
Not mentioned changes:
updatescoresbeatmapstatusandupdatescoressubmittedstatusin favour of all user scores recalculation.SunriseMetrics.RequestReturnedErrorCounterIncis being slowly sunset in favour of regularLog.WarningTimeElapsedfor score entity.There are the following things that are Out-Of-Scope of this PR, but planned to be implemented after it:
LocalPropertiesis a good example of that)I may have missed something else, but the list is quite big now. This is why all of these changes are going to the 0.2.0 release. No ETAs as always :)
cP8SbK31U5elIBU6.mp4