I1169 Deliver run testcases as they happen to clients reading the event feed - #1279
Conversation
Small changes to many modules to correctly handle runChanged() events - several of these should be fixed further to only handle the specific action they are expecting. Right now, added code to not do various updates of the action is a testcase result. Added new property to the RUN_STATUS packet (for the testcase ordinal). Adjusted PacketHandler and PacketFactory accordingly. Add configuration params for system.pc2.yaml to revert to sending all the testcases at the end of the judgment and to send a null judgment at the start of judging (*sigh*). This code has not been tested - this is the initial push.
Add more test cases to the sumit problem (there used to be only 1). Change problems to use STDIN as opposed to a file name. Fix bugs found during testing. Change sendToSpectatorsAndSites to sendToSpectatorsFeedersAndSites(). This is only used locally in PacketHandler to handle run status updates. Feeders should see these too.
For rejudgings, send testcase in batch at the end, like before. However, note that we changed the order so the testcases ("runs') will come out before the final judgment.
Only the VERY FIRST judging of a submission will generate real time testcase results ('runs") as they happen.
Added very detailed commenting of how the code works.
The judgements notification must be sent before the runs to preserve referential integrity (the judgement_id must be defined before referenced).
Forgot to add new virtual methods to NullController and MockController.
There was yet another Controller implementation that needed updating with the new methods.
This comment was marked as resolved.
This comment was marked as resolved.
Whenever you change system.pc2.yaml, you have delete the current contest files, and reload a new contest. Those settings are only used when loading a clean contest. Changing the system.pc2.yaml, then restarting will have no effect. DEL in a judgment means that the contest wasn't running when the run was judged (that is, it won't count toward the team's score). AC simply means if the contest was running the judgment would be accepted. you want to Start the contest before running tests. |
clevengr
left a comment
clevengr
left a comment
There was a problem hiding this comment.
I reviewed all (45!) changed files. I made a few minor comments on several of them, most of which can be ignored (author's choice). However, there are several places where I made meaningful comments -- some of which may have to do (I think) with my lack of understanding of precisely how the PR is intended to work.
In addition, there are three changed files where I do not understand the changes at all (src/edu/csus/ecs/pc2/clics/API202306/EventFeedStreamer.java, src/edu/csus/ecs/pc2/core/model/InternalContest.java, and src/edu/csus/ecs/pc2/core/PacketHandler.java); I'll file separate comments on those.
clevengr
left a comment
clevengr
left a comment
There was a problem hiding this comment.
I do not understand the changes made to src/edu/csus/ecs/pc2/clics/API202306/EventFeedStreamer.java; I think I need to have a conversation with the author.
clevengr
left a comment
clevengr
left a comment
There was a problem hiding this comment.
I do not understand the removal of the three lines (around 1670) in src/edu/csus/ecs/pc2/core/model/InternalContest.java.
clevengr
left a comment
clevengr
left a comment
There was a problem hiding this comment.
As stated in a prior comment, I'm confused about the changes around line 3188 in src/edu/csus/ecs/pc2/core/PacketHandler.java. I don't understand the underlying intent of the PR in this regard; I think I need to have a conversation with the author.
The idea is: if we are not sending test case results as they occur, it behaves like before, that is, when it receives a notification with the final judgment, it will send out the judgment, THEN all the run test cases. (Lines 562-575) if we are sending test case results as they occur, then it will send out a The The comments in the code, coupled with the above should make it clearer what is going on. In addition, the configuration flags are tested to control if individual test case results are sent. ( We can discuss at your leisure. |
|
@johnbrvc This doesn't seem right to me. It correctly outputs the updated "TLE" judgement, but then it also outputs a "runs" event containing judgement "AC". I was doing all of this using the |
Yeah, that does not seem right, however, I would suspect that it does the same thing on regular PC^2 as well (not using the PR). I will check. |
There was a problem hiding this comment.
During my runtime tests of the PR, I performed one test as follows:
- I edited the
system.pc2.yamlfile so that instead of the defaults specified by the PR changes it contained:
batch_testcases_on_event_feed: true
send_begin_judgment_on_event_feed: true
(the difference being that the first line was changed to true from its default of false).
I then ran the Steps to test the PR as specified.
On the event feed I got:
{"type":"submissions","token":"pc2-46","id":"1","data": {"id":"1","language_id":"cpp","problem_id":"sumit","team_id":"1","time":"2026-07-23T15:35:09.239-07","contest_time":"00:02:49.731","entry_point":null,"files":[{"href":"/contests/SumH/submissions/1/files","filename":"1.zip","hash":null,"mime":"application/zip"}]}}
{"type":"judgements","token":"pc2-47","id":"Run-3466769724683919556","data": {"id":"Run-3466769724683919556","submission_id":"1","judgement_type_id":"AC","start_time":"2026-07-23T15:35:09.450-07","start_contest_time":"00:02:49.924","end_time":"2026-07-23T15:35:11.303-07","end_contest_time":"00:02:51.777","max_run_time":0.016}}
{"type":"runs","token":"pc2-48","id":"TestCase-3817760598047810668","data": {"id":"TestCase-3817760598047810668","judgement_id":"Run-3466769724683919556","ordinal":1,"judgement_type_id":"AC","time":"2026-07-23T15:35:10.500-07","contest_time":"00:02:50.973","run_time":0.067}}
{"type":"runs","token":"pc2-49","id":"TestCase--7704976196617855965","data": {"id":"TestCase--7704976196617855965","judgement_id":"Run-3466769724683919556","ordinal":2,"judgement_type_id":"AC","time":"2026-07-23T15:35:10.610-07","contest_time":"00:02:51.083","run_time":0.018}}
{"type":"runs","token":"pc2-50","id":"TestCase--956987229973535317","data": {"id":"TestCase--956987229973535317","judgement_id":"Run-3466769724683919556","ordinal":3,"judgement_type_id":"AC","time":"2026-07-23T15:35:10.721-07","contest_time":"00:02:51.194","run_time":0.017}}
{"type":"runs","token":"pc2-51","id":"TestCase--8315747617535842765","data": {"id":"TestCase--8315747617535842765","judgement_id":"Run-3466769724683919556","ordinal":4,"judgement_type_id":"AC","time":"2026-07-23T15:35:10.832-07","contest_time":"00:02:51.305","run_time":0.016}}
{"type":"runs","token":"pc2-52","id":"TestCase-4545006008921113308","data": {"id":"TestCase-4545006008921113308","judgement_id":"Run-3466769724683919556","ordinal":5,"judgement_type_id":"AC","time":"2026-07-23T15:35:10.934-07","contest_time":"00:02:51.407","run_time":0.016}}
{"type":"runs","token":"pc2-53","id":"TestCase--3686643062479133521","data": {"id":"TestCase--3686643062479133521","judgement_id":"Run-3466769724683919556","ordinal":6,"judgement_type_id":"AC","time":"2026-07-23T15:35:11.045-07","contest_time":"00:02:51.518","run_time":0.016}}
{"type":"runs","token":"pc2-54","id":"TestCase--7405859407175628848","data": {"id":"TestCase--7405859407175628848","judgement_id":"Run-3466769724683919556","ordinal":7,"judgement_type_id":"AC","time":"2026-07-23T15:35:11.149-07","contest_time":"00:02:51.622","run_time":0.016}}
It appears to have (correctly) batched the runs events, BUT: there is no "begin judgement" event on the event feed (as there should be according to the system.pc2.yaml settings; rather, there is a single judgements event which already has a judgement ("AC"). This doesn't seem correct to me (or at least, it's not consistent with the phrase send_begin_judgment_on_event_feed: true), since I'm not seeing a "begin judgement" event with no judgements field in it.
And I have a related question: why do we even need two different YAML directives to deal with the changes this PR is trying to accomplish? Wouldn't a single directive that specifies either "do it the old way" or "do it the new way" be sufficient? It's not clear to me why we need four combinations of "how to do it".
In particular, for example, I tried testing with the combination:
batch_testcases_on_event_feed: false
send_begin_judgment_on_event_feed: false
The resulting EF looked like:
{"type":"submissions","token":"pc2-45","id":"1","data": {"id":"1","language_id":"cpp","problem_id":"sumit","team_id":"1","time":"2026-07-23T16:32:09.058-07","contest_time":"00:01:42.210","entry_point":null,"files":[{"href":"/contests/SumH/submissions/1/files","filename":"1.zip","hash":null,"mime":"application/zip"}]}}
{"type":"judgements","token":"pc2-46","id":"Run--1713066864958116477","data": {"id":"Run--1713066864958116477","submission_id":"1","judgement_type_id":"AC","start_time":"2026-07-23T16:32:09.330-07","start_contest_time":"00:01:42.472","end_time":"2026-07-23T16:32:10.866-07","end_contest_time":"00:01:44.008","max_run_time":0.018}}
{"type":"runs","token":"pc2-47","id":"TestCase-4728733945392224966","data": {"id":"TestCase-4728733945392224966","judgement_id":"Run--1713066864958116477","ordinal":1,"judgement_type_id":"AC","time":"2026-07-23T16:32:09.981-07","contest_time":"00:01:43.073","run_time":0.056}}
{"type":"runs","token":"pc2-48","id":"TestCase-4891886358027203752","data": {"id":"TestCase-4891886358027203752","judgement_id":"Run--1713066864958116477","ordinal":2,"judgement_type_id":"AC","time":"2026-07-23T16:32:10.132-07","contest_time":"00:01:43.224","run_time":0.024}}
{"type":"runs","token":"pc2-49","id":"TestCase-7499777627633890770","data": {"id":"TestCase-7499777627633890770","judgement_id":"Run--1713066864958116477","ordinal":3,"judgement_type_id":"AC","time":"2026-07-23T16:32:10.299-07","contest_time":"00:01:43.391","run_time":0.029}}
{"type":"runs","token":"pc2-50","id":"TestCase-8333069458314317264","data": {"id":"TestCase-8333069458314317264","judgement_id":"Run--1713066864958116477","ordinal":4,"judgement_type_id":"AC","time":"2026-07-23T16:32:10.439-07","contest_time":"00:01:43.531","run_time":0.02}}
{"type":"runs","token":"pc2-51","id":"TestCase-3788176202315816000","data": {"id":"TestCase-3788176202315816000","judgement_id":"Run--1713066864958116477","ordinal":5,"judgement_type_id":"AC","time":"2026-07-23T16:32:10.561-07","contest_time":"00:01:43.653","run_time":0.019}}
{"type":"runs","token":"pc2-52","id":"TestCase--1807112542352270722","data": {"id":"TestCase--1807112542352270722","judgement_id":"Run--1713066864958116477","ordinal":6,"judgement_type_id":"AC","time":"2026-07-23T16:32:10.679-07","contest_time":"00:01:43.771","run_time":0.017}}
{"type":"runs","token":"pc2-53","id":"TestCase-7430864510850296993","data": {"id":"TestCase-7430864510850296993","judgement_id":"Run--1713066864958116477","ordinal":7,"judgement_type_id":"AC","time":"2026-07-23T16:32:10.799-07","contest_time":"00:01:43.891","run_time":0.018}}
But this is precisely the same as the "old behavior" -- so I can't see why we need that combination, nor the combination:
batch_testcases_on_event_feed: true
send_begin_judgment_on_event_feed: false
This is not a new issue. This is the way it currently works in the develop branch (production). I can fix it with this PR I suppose, or create a new issue, fix it, and close both issues when the PR is approved. Technically, as Doug would say, "Out of scope". |
My suggestion would be to create a new issue, since it is indeed "out of scope" for this PR... |
Whether right or wrong, this is "by design". The Of course, I can (easily) change that behavior and make it always check BOTH flags, even in so-called "legacy" mode (non-batch mode). Or, as you suggest, get rid of the SendBeginJudgmentOnEF flag and always send a null judgment if in batch mode. |
Add comments to better explain test case indexes. Add comments to better explain that sendToFeeders means CLICS Event Feeders. Rename enums to more accurately explain their function: RUN_TESTCASE_RESULT->RUN_TESTCASE_COMPLETED, TESTCASE_RESULT->TESTCASE_COMPLETED.
A new issue has been created for this, but I think the action will be "do nothing". #1283 |
It was decided we do not need this flag. If RunTestCase results are sent as they occur, we will now always send a null judgment notification first (to maintain referential integrity which is required by the CLICS specification. If we are batching RunTestCase results and sending them after the judgment for the run completes, (the way PC2 used to always operate), then we never send a null judgment notification.
The SendBeginJudgmentOnEF flag has been removed. null judgment notifications are always sent if sending RunTestCase results as they occur. null judgment notifications are never sent if in batch mode (sending RunTestCase results AFTER the final judgment is sent.) Addressed in commit: 2ac1e8d |
clevengr
left a comment
clevengr
left a comment
There was a problem hiding this comment.
I have reviewed the latest changes, and they all make sense. I have performed multiple runtime tests including using both batch_testcases_on_event_feed: false and batch_testcases_on_event_feed: true. All tests worked as expected.
However, I noted the following issues:
- The file
ContestSnakeYamlLoaderwas (properly) updated to no longer read a config valueSEND_BEGIN_JUDGMENT_ON_EF, since that config value is no longer defined or meaningful. However,IContestLoaderstill contains a definition for that value:
String SEND_BEGIN_JUDGMENT_ON_EF = "send_begin_judgment_on_event_feed";
To avoid possible future confusion I think IContestLoader should be updated by removing that definition, since it's no longer defined or being used for anything.
- File
samps/contests/clics_sumithello/config/system.pc2.yamlstill contains the following lines:
# Default is true, but include it anyway
send_begin_judgment_on_event_feed: true
To avoid future confusion I think those lines should be removed since there's no longer any such configuration value defined.
Removed the definition for SEND_BEGIN_JUDGMENT_ON_EF in IContestLoader. Removed send_begin_judgment_on_event_feed in clics_sumithello sample pc2v9.ini.
clevengr
left a comment
clevengr
left a comment
There was a problem hiding this comment.
I've reviewed the latest code, and performed all the tests suggested in the PR (plus a few of my own). Everything looks good; I approve the PR.
3 participants
Description of what the PR does
Optionally send CLICS (Version 2023-06 and greater) "
runs" testcase results on the event feed as they are judged instead of waiting until all testcases have been judged and the final judgment is sent. This is primarily for downstream event feed clients that would like to render testcase results visually as they occur (eg. ICPCLive).The use of this functionality is controlled by 2 new configuration parameters that may be specified in the
system.pc2.yamlfile.batch_testcases_on_event_feed- boolean value - Default value: falseruns") as the happen in real-time instead of at the end of the submission's judgment.send_begin_judgment_on_event_feed- boolean value - Default value: truejudgements" notification prior to starting the judging of a submission. This notification will not contain ajudgement_type_id, nor will it contain the end times ormax_run_timeproperties. The purpose of sending this is to let downstream clients know that "runs" notifications are about to happen for a submission in addition, it provides referential integrity, as required by the CLICS specification, since thejudgement_idis included in the "runs" notifications, so, thejudgmentsrecord must be sent before using its ID.judgements" notification when judging of the submission is complete, followed by the "runs" notifications. The "runs" must be sent after the "judgements" notification to provide referential integrity.Delivering of testcase results as they occur is only supported for the initial judging of a submission. Any re-judging or manual judging will cause the testcase results ("runs") to be sent after the final judgement is complete with a valid
judgement_type_id.An example event feed snippet of using the default settings for the new configuration flags above would be:
Issue which the PR addresses
Fixes #1169
Environment in which the PR was developed (OS,IDE, Java version, etc.)
java version "1.8.0_321"
Java(TM) SE Runtime Environment (build 1.8.0_321-b07)
Java HotSpot(TM) 64-Bit Server VM (build 25.321-b07, mixed mode)
and
Ubuntu 24.03.1 with Java 21.0.4
Precise steps for testing the PR (i.e., how to demonstrate that it works correctly)
This PR supplies additions to the sample clics_sumithello contest (
samps/contests/clics_sumithello). These additions include fixing theproblem.yamlfiles for both problems toreadFromSTDINset to true, adding the default setting of the new configuration parameters in thesystem.pc2.yamlfile and adding more test cases to the sumit problem (there used to be just one test case which isn't very useful testing most things, including this PR.)pc2serveris now running)curlcommand to monitor the event feed:curl -k https://administrator1:administrator1@localhost:50443/contests/SumH/event-feedsamps/contests/clics_sumithello/configfolder:sumit/submissions,hello/submissions,sumit/submissions/accepted,hello/submissions/accepted.samps/src/hello.cppto thesamps/contests/clics_sumithello/config/hello/submissions/acceptedfolder.samps/src/isumit.cppto thesamps/contests/clics_sumithello/config/sumit/submissions/acceptedfolder.curlcmd window, you should see the following:Things to observe:
judgement_type_idproperty (and no end times properties).ordinal" property).judgement_type_id).The above instructions should provide you with enough information now to perform other tests, such as trying the "hello" sample. In addition, you can copy additional source files into the
submissions/acceptedfolder and try them out too. You'll have to restart the administrator client if you add things to thesubmissions/acceptedfolder.Some other tests you may want to try:
system.pc2.yamlfile to set the new configuration options to their opposite values so that PC2 will work the way it used to. Then, repeat the entire process above and note that the runs notifications come after the final judgment, and, there is no initial null judgment.QUEUED FOR COMPUTER JUDGEMENT", or by Rejudging it using the administrator client (select the run from the Runs tab, and press Rejudge). Remember that rejudging will not send out runs notifications as they happen, they will appear at the end of the judgement. Only the very first judging of a submission will use the new functionality.