Skip to content

Conversation

meisterT
Copy link
Member

@meisterT meisterT commented Oct 3, 2025

Fixes #3069.

@meisterT meisterT requested a review from eldering October 4, 2025 09:37
Copy link
Member

@eldering eldering left a comment

Choose a reason for hiding this comment

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

Just to understand how this is cause for the bug (and probably good to add a summary to the commit): this endpoint is called every time that a judgehost checks in or only when it connects the first time on startup?
And shouldn't giving back these judgings happen when the rejudging is cancelled?

@meisterT
Copy link
Member Author

meisterT commented Oct 4, 2025

Good question, I reworded the commit, let me know if this answers everything.

@vmcj
Copy link
Member

vmcj commented Oct 4, 2025

The third paragraph doesn't read nice in the commit.
Previously, we did this for incorrectly assume that active rejudgings are marked as valid

judgedaemons check in after start-up and after failed judgetask, see
https://github.com/DOMjudge/domjudge/blob/2222a867f99275cbdb8e81d4681e47522a29669f/judge/judgedaemon.main.php#L984

Checking in has the side effect that judgetasks that no longer need to
be executed are given back.

Previously, we checked whether either the judging or the rejudging was
valid, assuming that active rejudgings are marked as valid. This is not
true for repeated rejudgings where only the last instance is "valid".
For the check, it doesn't matter whether a rejudging is valid, only
whether a judging is associated with one.

Fixes DOMjudge#3069.
@meisterT
Copy link
Member Author

meisterT commented Oct 4, 2025

Thanks, fixed.

@eldering
Copy link
Member

eldering commented Oct 5, 2025

From the commit message now:

judgedaemons check in after start-up and after failed judgetask, see

// Potentially return remaining outstanding judgetasks here.

so I understand that the judge function returns true only when the submission was correct, so this should always mark these unfiinished judging runs as done? Makes sense, but I can't say I 100% follow the details in code.

But it's fine, no objections to merging.

@meisterT
Copy link
Member Author

meisterT commented Oct 5, 2025

It returns true when it needs more work. I will see how to add some comments in judgedaemon to make this more clear.

@meisterT meisterT added this pull request to the merge queue Oct 5, 2025
Merged via the queue into DOMjudge:main with commit 3b5b815 Oct 5, 2025
36 checks passed
@meisterT meisterT deleted the rep_rej branch October 5, 2025 14:10
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.

Judging runs shown in pending state after doing a repeated rejudging

3 participants