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

ProxyService support for different contest RWS #1102

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

PPakalns
Copy link
Contributor

@PPakalns PPakalns commented Jan 22, 2019

ProxyService only supports one parallel contest when sending initial data to listed RWS. After that, it sends all submission changes (even from different contests). These changes are discarded by RWS because of data inconsistency.

This pull request adds support to ProxyService for sending scores to multiple RWS where each of them can show a different subset of contests.
This request fixes the problem where ProxyService sends unrelated contest submissions to RWS.

Additionally, this allows to easily set up RW servers for multiple contests that are run in parallel and keep their data unmerged if needed.


This change is Reviewable

@codecov
Copy link

codecov bot commented Jan 22, 2019

Codecov Report

Merging #1102 into master will decrease coverage by 0.09%.
The diff coverage is 42.62%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #1102     +/-   ##
=========================================
- Coverage   62.02%   61.92%   -0.1%     
=========================================
  Files         230      230             
  Lines       16587    16598     +11     
=========================================
- Hits        10288    10279      -9     
- Misses       6299     6319     +20
Flag Coverage Δ
#functionaltests 45.68% <37.7%> (-0.17%) ⬇️
#unittests 43.3% <34.42%> (+0.04%) ⬆️
Impacted Files Coverage Δ
cms/service/ResourceService.py 59.06% <0%> (+1.07%) ⬆️
cms/conf.py 84.73% <100%> (ø) ⬆️
cms/service/ProxyService.py 60.09% <42.37%> (+0.52%) ⬆️
cms/grading/Job.py 83.88% <0%> (-4.27%) ⬇️
cms/service/EvaluationService.py 65.15% <0%> (-3.2%) ⬇️
cms/io/service.py 68.29% <0%> (-1.22%) ⬇️
cms/server/admin/handlers/dataset.py 24.76% <0%> (-0.93%) ⬇️
cms/db/util.py 50.37% <0%> (-0.75%) ⬇️
cms/grading/Sandbox.py 67.87% <0%> (-0.73%) ⬇️
cms/db/filecacher.py 77.7% <0%> (-0.33%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 584cc42...f311d0a. Read the comment docs.

@cms-dev cms-dev deleted a comment Jan 22, 2019
@cms-dev cms-dev deleted a comment Jan 22, 2019
@cms-dev cms-dev deleted a comment Jan 22, 2019
@cms-dev cms-dev deleted a comment Jan 22, 2019
service.name == "ResourceService" or \
(self.contest_id is None and
service.name == "ProxyService"):
if service.name in ("LogService", "ResourceService"):
Copy link
Member

Choose a reason for hiding this comment

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

It should be more conventional to use a list here.

"_help": "RWSs where the scores are to be sent. Don't include the",
"_help": "load balancing proxy (if any), just the backends. If any",
"_help": "of them uses HTTPS specify a file with the certificates",
"_help": "you trust.",
"rankings": ["http://usern4me:passw0rd@localhost:8890/"],
"rankings": [[1, "http://usern4me:passw0rd@localhost:8890/"]],
Copy link
Member

Choose a reason for hiding this comment

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

This somewhat breaks the current workflow, because now you have to update cms.conf every time your contests change.

I suppose that the real fix would be to make RWS contest-aware, that is, serve multiple results pages under a prefix when multi-contest mode is on, similarly to how CWS works now. (Although this would interfere with RWS's feature of joining "day 1" and "day 2" contests…)

Maybe we could use a default of 0 or -1 here to indicate that the old behavior should be used, so that users with existing workflows don't need to do anything. I would appreciate opinions on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally, with change to "rankings" property format in config file, cms/cmscontrib/RWSHelper.py retrieving of ranking server urls needs to be fixed.

@wil93
Copy link
Member

wil93 commented Feb 10, 2019

Did you consider changing the DB model so that a contest can provide a ranking_url as well as other fields already provided like title and description? If the field is null we can fallback to the ranking specified in cms.conf

In this way, proxyservice could send contest data to the right URL for each contest.

If there are multi-day contests we can easily support that by setting the same ranking_url value for both days

@andreyv
Copy link
Member

andreyv commented Feb 10, 2019

Or leave the upload URL in the config file, but for the contest specify a "contest group" it belongs to, and use this group as the RWS prefix. Empty group would mean the current behavior. Very nice idea, @wil93 :)

@andreyv
Copy link
Member

andreyv commented Feb 21, 2019

There is one more problem with multiple RankingWebServers, if you run them on the same machine. Not taking into account that each needs a different port number, they also need different log and data directories. By default they all try to use /var/local/log/cms/ranking and /var/local/lib/cms/ranking.

This can be worked around by specifying different log_dir and lib_dir settings and having a separate config file for each RWS instance (overridden using CMS_RANKING_CONFIG).

@andreyv
Copy link
Member

andreyv commented May 1, 2019

To expand on the last idea:

We can add a new "group" field to the contest in DB. Contests in one group would be temporally related, for example, day 1 and day 2. When ProxyService sends data to RWS, it would also send the contest's group along the data.

When RWS receives the data, it would display it under the /<group> URL prefix. An empty group would mean the current behavior.

For example, for the Latvian olympiad with two days and junior/senior student groups, we would have

Contest       Group
===========   ======
junior_day1   junior
senior_day1   senior
junior_day2   junior
senior_day2   senior

and there would be two scoreboards at http://rws/junior and http://rws/senior.

This way there is no need to change cms.conf/cms.ranking.conf each time. Also, as shown above, running multiple RWSs is troublesome too.

A separate question is how to deal with the leaking of the task names from the second day. I think this can be handled in a different issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants