Send less event updates for judgements in parallel mode. #2523
Conversation
We used to send event updates for all testcases that came in after we already determined the final verdict for a problem. Now we check if we already have a previous end time for the judgement, and then we don't do this anymore. Note that race conditions can still make it such that we send multiple update events, where two judgehosts know the result at the same time and. However, this happens way less often.
|
Does it hurt? |
|
@johnbrvc does it hurt for you, more than that you run a codepath to check if the judgement changed? I wonder if this is a CLICS discussion as well, can a judgement change after an |
|
IIRC, we did this intentionally to make the event feed and endpoints match. Cc @eldering |
Yeah this is what I'm thinking as well, hence my last sentence. |
|
It doesn't hurt per se, but it is quite confusing, and pollutes the logs
(in addition, it bloats the event feed). Which judgement for a specific
judgement_id are we supposed to believe? the first? the 20th? the
last?
It seems that once a judgement_type_id is set (WA/AC, etc), then that's
it, unless the judge_type_id changes (which does not happen very often, if
at all - manual intervention).
I'm not sure what use anything after that is? The "runs" entries have each
test case's disposition and execution time, so why does the final judgement
record update?
…On Wed, May 1, 2024 at 10:28 AM Nicky Gerritsen ***@***.***> wrote:
@johnbrvc <https://github.com/johnbrvc> does it hurt for you, more than
that you run a codepath to check if the judgement changed?
I wonder if this is a CLICS discussion as well, can a judgement change
after an end_time has been set?
—
Reply to this email directly, view it on GitHub
<#2523 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABLFGEYP74MV4PAGLYBFXYLZAD3YBAVCNFSM6AAAAABHCBD5O2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBYGU2DKNRWGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
You could decide to only log if there is any contradicting information in the update.
The last as we are sending updated information. I think this is in line with https://ccs-specs.icpc.io/draft/contest_api#general-requirements which specifically allows duplicate events and "when an object changes one or more times a notification will be sent" as well "the latest notification sent for any object is the correct and current state of that object."
Technically something was judged after the previously recorded endtime, so we update it to reflect that. With that it is exposed in the specific API endpoint. To not have conflicting information in the feed and the API, we also send it in the feed. |
I guess this is my question. How can you have something that was already judged (and given a judgment (WA/AC/TLE, etc.), be judged again, the same way? It seems (IMHO) that the judgment record should be sent when the judging is complete, or, don't include the It doesn't matter to me (or PC2, for that matter). I was just pointing it out is all... |
That's just the nature of the first incorrect test case (in order) determines the final verdict. When that is known, we let teams and downstream clients know although there is still judging going on. But these remaining test cases cannot influence the final verdict. |
I agree with @johnbrvc that it doesn't really make sense to update the judgement after it has a verdict. Even though it is still running on our side, that really should/does not change the judgement anymore. I think what we should do, is simply still send the runs, not the judgements anymore. Since these don't affect the judgement, there's no need to update our judgement end time (either internally or externally), so we don't have to send out an event for that. Indeed, otherwise we should have, since the event feed should match the contents of the API endpoints. |
|
Note that we also update the max run time to be the max of all runs. This then also needs to change to only take the max of runs that added to the verdict, which might be hard. If we run in parallel, we might already have run some later test cases which would then be in the max run time during running but not anymore after it is done. |
We used to send event updates for all testcases that came in after we already determined the final verdict for a problem. Now we check if we already have a previous end time for the judgement, and then we don't do this anymore.
Note that race conditions can still make it such that we send multiple update events, where two judgehosts know the result at the same time. However, this happens way less often.
Do we still want to update the end time? We currently do but then never send an update event anymore, even though the end time changed. So is this actually valid? Or do we not want to merge this and accept that we send updates (in Luxor we had submissions with 47 updates, while only 1 is really needed)? Or maybe in lazy mode we do not take into account the run time of any testcase that came after the first wrong one?
Found by @johnbrvc.