Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rank design and implement, in progress. #99

Merged
merged 8 commits into from
Sep 4, 2024
Merged

Rank design and implement, in progress. #99

merged 8 commits into from
Sep 4, 2024

Conversation

Zztrans
Copy link
Member

@Zztrans Zztrans commented Aug 25, 2024

Some design explain

I just test one user, one problem, one judger case. The scoreCache result is right as I think.

scoreCache is almost finished.

rankCache will be implemented in the nearest future.

Some advice will be appreciated.

@Zztrans Zztrans requested a review from slhmy August 25, 2024 16:24
Copy link

codecov bot commented Aug 25, 2024

Codecov Report

Attention: Patch coverage is 57.14286% with 63 lines in your changes missing coverage. Please review.

Project coverage is 31.47%. Comparing base (4fce239) to head (34b5f46).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
services/judge/judge_rank.go 0.00% 20 Missing ⚠️
services/judge/judge_cache.go 73.33% 6 Missing and 6 partials ⚠️
models/judge/judge_db.go 0.00% 11 Missing ⚠️
models/judge/judge_rank_db.go 71.42% 3 Missing and 3 partials ⚠️
models/judge/judge_rank_cache.go 0.00% 5 Missing ⚠️
models/judge/judge_rank_cache_db.go 81.81% 2 Missing and 2 partials ⚠️
services/user/user.go 0.00% 3 Missing ⚠️
models/judge/judge_score_cache_db.go 85.71% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #99      +/-   ##
==========================================
+ Coverage   27.97%   31.47%   +3.50%     
==========================================
  Files          35       42       +7     
  Lines        1076     1223     +147     
==========================================
+ Hits          301      385      +84     
- Misses        729      780      +51     
- Partials       46       58      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Zztrans Zztrans requested a review from akamya997 August 25, 2024 16:35
@@ -53,6 +63,8 @@ func clearDB() {
&problem_model.ProblemTag{},
&judge_model.Judge{},
&judge_model.JudgeResult{},
&judge_model.ScoreCache{},
"problem_tags",
Copy link
Member

Choose a reason for hiding this comment

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

Duplicated with &problem_model.ProblemTag{}

ProblemSlug string `json:"problemSlug" gorm:"primaryKey"`
Problem problem_model.Problem `json:"problem"`
SubmissionCount int64 `json:"submissionCount" gorm:"default:1"` // judge create time < solvetime will be count
IsCorrect bool `json:"isCorrect" gorm:"default:false"`
Copy link
Member

Choose a reason for hiding this comment

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

Perfer IsAccepted here

gorm_agent "github.com/oj-lab/oj-lab-platform/modules/agent/gorm"
)

func UpdateScoreCache(ctx context.Context, uid uuid.UUID, verdict judge_model.JudgeVerdict) (*judge_model.ScoreCache, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Perfer UpsertScoreCache here

scoreCache.IsCorrect = true
scoreCache.SolveTime = judge.CreateAt
}
newScoreCache, err := judge_model.CreateJudgeScoreCache(db, scoreCache)
Copy link
Member

Choose a reason for hiding this comment

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

CreateJudgeScoreCache may failed here because of concurrency conflict.
If failed, get data again and continue the update logic.

)

// user contest problem summary score info
type ScoreCache struct {
Copy link
Member

Choose a reason for hiding this comment

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

Perfer JudgeScoreCache here, naming should not differed in service or db modules.

@Zztrans
Copy link
Member Author

Zztrans commented Aug 28, 2024

I update the rankcache in the time scoreCache update.

I found the logic may be too complicated in a single function.

This impl maybe fast, which update only when needed.

@Zztrans
Copy link
Member Author

Zztrans commented Aug 28, 2024

More test and final rank query api and service in progress.

@@ -13,7 +14,10 @@ import (

func CreateUser(ctx context.Context, user user_model.User) (*user_model.User, error) {
db := gorm_agent.GetDefaultDB()

_, err := judge_model.CreateJudgeRankCache(db, judge_model.NewJudgeRankCache(user.Account))
Copy link
Member

Choose a reason for hiding this comment

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

If RankCache is necessary for every user, using relationship in GORM will be better, so that we won't need to create RankCache explicitly.

Copy link
Member

Choose a reason for hiding this comment

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

I'm also welcome the design of lazy creation for RankCache, this depends on whether we think RankCache is needed for user who don't submit any judges.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now it's lazy creation I think.

@Zztrans
Copy link
Member Author

Zztrans commented Sep 4, 2024

image
image

- simplify upsert logic
- judge_rank api service model db
- some test
@Zztrans Zztrans requested a review from slhmy September 4, 2024 09:02
@slhmy slhmy merged commit 116ffa8 into main Sep 4, 2024
4 checks passed
@Zztrans Zztrans deleted the zztrans/rank branch September 28, 2024 14:34
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