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

Add support for ephemeral services. #1302

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

veluca93
Copy link
Collaborator

@veluca93 veluca93 commented Dec 6, 2024

Ephemeral services are services that are not fixed in the configuration file, but dynamically added as they connect. This is especially useful in a setup in which cmsWorker/cmsContestWebServer are scaled dynamically, as one might do when configuring CMS for running on cloud services.

@veluca93 veluca93 requested a review from wil93 December 6, 2024 10:29
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 71.15385% with 30 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@b22e14c). Learn more about missing BASE report.
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
cms/conf.py 60.41% 19 Missing ⚠️
cms/util.py 65.00% 7 Missing ⚠️
cms/service/workerpool.py 76.92% 3 Missing ⚠️
cms/server/contest/server.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1302   +/-   ##
=======================================
  Coverage        ?   68.01%           
=======================================
  Files           ?      332           
  Lines           ?    26623           
  Branches        ?        0           
=======================================
  Hits            ?    18108           
  Misses          ?     8515           
  Partials        ?        0           
Flag Coverage Δ
functionaltests 46.05% <58.58%> (?)
unittests 55.51% <48.07%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@veluca93 veluca93 force-pushed the ephemeral-services branch 2 times, most recently from ea1c381 to cb157a2 Compare December 6, 2024 20:09
@veluca93 veluca93 force-pushed the ephemeral-services branch 2 times, most recently from ede3e52 to 3f74ac4 Compare December 6, 2024 21:06
Copy link
Member

@wil93 wil93 left a comment

Choose a reason for hiding this comment

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

I suggest expanding the documentation to add some information about this new kind of service.

@wil93 wil93 changed the base branch from master to main December 26, 2024 19:18
edomora97 and others added 2 commits February 26, 2025 16:42

Partially verified

This commit is signed with the committer’s verified signature.
Virv12’s contribution has been verified via GPG key.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
Ephemeral services are services that are not fixed in the configuration
file, but dynamically added as they connect. This is especially useful
in a setup in which cmsWorker/cmsContestWebServer are scaled
dynamically, as one might do when configuring CMS for running on cloud
services.

Verified

This commit was signed with the committer’s verified signature.
Virv12 Filippo Casarin
@Virv12 Virv12 force-pushed the ephemeral-services branch from b183ced to c39d658 Compare February 26, 2025 15:43
@Virv12
Copy link
Contributor

Virv12 commented Feb 26, 2025

@wil93 I added a few lines of doc, it's not much but there is not much to say.

@wil93
Copy link
Member

wil93 commented Feb 26, 2025

I guess the updates to the cms.conf.sample file can also be considered part of the documentation, in a way. However I am still worried it might be unclear how to this new feature works... Some questions:

Is "not providing a shard number" the one and only way to opt in to ephemeral services? If I have both the static and the ephemeral configuration in place in cms.conf, what happens if I launch cmsResourceService -a? Also, can any service can be ephemeral, or is it just Worker and ContestWebServer? If it's any service, couldn't we simplify the definition to something like the following?

"ephemeral_services":
    {
            "subnet": "127.0.0.0/8",
            "min_port": 26000,
            "max_port": 26999
    },

For the contest_listen_address we now say "When using ephemeral services only the first address and port are used", what does this mean?

@wil93
Copy link
Member

wil93 commented Feb 26, 2025

Also, and this could be a bad idea, but would it be possible to introduce ephemeral services as more "clean cut" way of launching things, i.e. force our users to choose either "all ephemeral" or "old way", instead of letting our users run CMS with a mix of ephemeral and non-ephemeral? Would it make sense?

Verified

This commit was signed with the committer’s verified signature.
Virv12 Filippo Casarin
@Virv12
Copy link
Contributor

Virv12 commented Mar 6, 2025

Also, can any service can be ephemeral, or is it just Worker and ContestWebServer?

Any service can be ephemeral, but only cws and worker have been tested.

If it's any service, couldn't we simplify the definition to something like the following?

I guess we could, we will lose the possibility the have different services in different subnets but I guess it's not a big problem.

Also, and this could be a bad idea, but would it be possible to introduce ephemeral services as more "clean cut" way of launching things, i.e. force our users to choose either "all ephemeral" or "old way", instead of letting our users run CMS with a mix of ephemeral and non-ephemeral? Would it make sense?

Could we? Yes. Does it make sense? I don't think, sometimes it's useful to mix the two kind of services, mostly to debug or quickly try something.

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.

None yet

4 participants