Add worker config options #1051

akihikokuroda · 2023-10-23T18:38:14Z

Summary

This PR is rework of PR #1025

Details and comments

Summary
expose number of workers option (number of workers, min workers and max workers when auto scaling is enabled)

Details and comments
The number of worker can be specified in the Configuration object for QuantumServerless.run function.

provider = ServerlessProvider(
username=os.environ.get("GATEWAY_USER", "user"),
password=os.environ.get("GATEWAY_PASSWORD", "password123"),
# token=os.environ.get("GATEWAY_TOKEN", ""), # token can be used instead of user/password combination
host=os.environ.get("GATEWAY_HOST", "http://localhost:8000"),
)
serverless = QuantumServerless(provider)
program = Program(
title="First program", entrypoint="program.py", working_dir="./source_files/"
)

job = serverless.run(program, config=Configuration(workers=2, min_workers=1, max_workers=5, auto_scaling=True))

akihikokuroda · 2023-10-23T18:38:57Z

I still need to work with tests for this but please take a look. Thanks!

psschwei

general question on workers vs. min/max workers : does one ever override the other (i.e. if we set workers, does that invalidate min/max scaling)?

I didn't check if there's a ray doc that covers this, so if there is feel free to tell me to go read the #$%#$%# docs 😄

psschwei · 2023-10-23T19:55:17Z

client/quantum_serverless/core/program.py

+    Args:
+        title: program name
+        entrypoint: is a script that will be executed as a job
+            ex: job.py
+        env_vars: env vars
+        dependencies: list of python dependencies to execute a program
+        working_dir: directory where entrypoint file is located (max size 50MB)
+        description: description of a program
+        version: version of a program


should the args here be workers, cpu, etc.?

The configuration is specified in run method of Quantum_serverless

serverless.run(program, config=Configuration(max_workers=12, min_workers=5)

I'll update the doc string of the function

akihikokuroda · 2023-10-23T20:05:25Z

@psschwei The workers is used when the auto scaling is not enabled. The min/max workers are used when the auto scaling is enabled.

Tansito · 2023-10-23T20:08:21Z

I was a bit out these last weeks so maybe this question is a bit dumb so sorry in advance (and maybe this may be discussed in the issue). Is it has sense to let the user to modify the min/max number of workers? And if so, I see valuable to have an option that prevent to do that in case the user doesn't want to allow that kind of behavior from external users. Opinions: @IceKhan13 , @psschwei , @akihikokuroda ? (maybe a dumb question, as I said 😅 ).

akihikokuroda · 2023-10-23T20:13:51Z

@Tansito The max allowed values for the workers are specified in the settings.py (read from env variables). So users are not allowed to set values bigger than those.

psschwei · 2023-10-23T20:14:21Z

let the user to modify the min/max number of workers

it came up during the internal tests

Tansito · 2023-10-23T20:18:32Z

@Tansito The max allowed values for the workers are specified in the settings.py (read from env variables). So users are not allowed to set values bigger than those.

I see so we are going to allow only modifications inside a threshold. Thanks @akihikokuroda ❤️

Happy to return to the work, I will try to catch-up and return 100% as soon as I can haha. Thanks team!

psschwei · 2023-10-23T20:35:18Z

@psschwei The workers is used when the auto scaling is not enabled. The min/max workers are used when the auto scaling is enabled.

so, for example, something like this

serverless.run(program, config=Configuration(workers=2, min_workers=1, max_workers=5, auto_scaling=True))

would ignore the workers=2 since auto_scaling is set to true, right?

We should probably add a doc on program configuration (to go along with the one for client configuration) to spell out all the options.

gateway/api/ray.py

gateway/api/schedule.py

IceKhan13 · 2023-10-23T20:49:44Z

gateway/api/views.py

@@ -86,11 +87,10 @@ def run(self, request):
        tracer = trace.get_tracer("gateway.tracer")
        ctx = TraceContextTextMapPropagator().extract(carrier=request.headers)
        with tracer.start_as_current_span("gateway.program.run", context=ctx):
-            serializer = self.get_serializer(data=request.data)


serializer should handle all related entities, so you do not need to created them.

I restore the use of the ProgramSerializer. Thanks!

gateway/main/settings.py

client/quantum_serverless/core/job.py

psschwei

Had one question (and one nit), but overall LGTM.
Will leave for @IceKhan13 to approve (as he had requested a few changes)

client/quantum_serverless/core/job.py

psschwei · 2023-10-26T13:26:38Z

gateway/api/ray.py

+                "workers": program_config.workers,
+                "min_workers": program_config.min_workers,
+                "max_workers": program_config.max_workers,
+                "auto_scaling": program_config.auto_scaling,


What happens if bad values are passed? For example, a user sets min=5 and max=2... what kind of error message would they see? Is that caught by Ray or do we need to catch it? (I haven't tried yet, but based on past experience users will pass all kinds of combinations of bad values when configuring...)

I tried it with max=1, min=2. Ray didn't care, it just created 1 worker.

akihikokuroda · 2023-10-26T15:04:57Z

I have to add the config to the existing program path.

psschwei · 2023-10-26T18:59:39Z

LGTM

Tansito · 2023-11-03T20:43:16Z

Agree to attach the configuration to a Job!

IceKhan13 · 2023-11-03T20:56:17Z

Woohoo! Config to Job connection flawless victory!

Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>

akihikokuroda · 2023-11-03T21:42:40Z

ProgramConfig has been changed to JobConfig and it is related to Job. Please review. Thanks!

IceKhan13 · 2023-11-06T14:04:05Z

I'm on it! Thank you Aki!

IceKhan13 · 2023-11-06T17:51:22Z

gateway/api/models.py

+        null=True,
+        validators=[
+            MinValueValidator(0),
+            MaxValueValidator(settings.RAY_CLUSTER_WORKER_REPLICAS_MAX),


configuration values in models will create conflict. If I change my settings now it will generate new migration diff and django will be complaining about migrations. Validation for this feilds should live in serializers

IceKhan13 · 2023-11-06T17:52:48Z

gateway/api/ray.py

+    user: Any,
+    cluster_name: Optional[str] = None,
+    cluster_data: Optional[str] = None,
+    job_config: Optional[JobConfig] = None,


we accept optional job_config here, but later on we always act like it is not None.

Here we should check for None and fall back to default from settings.

IceKhan13 · 2023-11-06T17:55:36Z

gateway/api/views.py

@@ -150,12 +157,18 @@ def run(self, request):
            if not serializer.is_valid():
                return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)

+            try:


If we replace it with

jobconfig = None config_data = request.data.get("config") if config_data: config_serializer = JobConfigSerializer(data=json.loads(config_data)) if not config_serializer.is_valid(): return Response(config_serializer.errors, status=status.HTTP_400_BAD_REQUEST) jobconfig = config_serializer.save()

then we resolve validation + errors will be in a nice understandable format

QuantumServerlessException: | Message: Http bad request. | Code: 400 | Details: {"workers":["Ensure this value is less than or equal to 5."]}

and we will not longer need save_jobconfig function.

IceKhan13

Almost there :)

Remaining things to fix:

remove settings variables from models and migrations
move validation to serializer ref
write tests for serializer and api view with configuration

akihikokuroda · 2023-11-06T19:15:22Z

@IceKhan13 Thanks for review. I'll make chages.

IceKhan13 · 2023-11-06T22:05:52Z

No, thank you for handling such a big change!

akihikokuroda · 2023-11-07T19:11:36Z

The runtime code changes are done. I'm working for the tests.

akihikokuroda · 2023-11-08T17:26:56Z

There remaining fixes have done. Please take a look! Thanks!

IceKhan13

LGTM!

Thank you @akihikokuroda for working on it!

akihikokuroda · 2023-11-08T23:53:26Z

Thanks!

IceKhan13 · 2023-11-13T14:56:57Z

Reopened #1085

Sorry messed up with branches

akihikokuroda requested review from IceKhan13, psschwei and Tansito October 23, 2023 18:38

akihikokuroda mentioned this pull request Oct 23, 2023

expose number of workers #1025

Closed

psschwei reviewed Oct 23, 2023

View reviewed changes

IceKhan13 reviewed Oct 23, 2023

View reviewed changes

gateway/api/ray.py Outdated Show resolved Hide resolved

IceKhan13 reviewed Oct 23, 2023

View reviewed changes

gateway/api/schedule.py Outdated Show resolved Hide resolved

IceKhan13 reviewed Oct 23, 2023

View reviewed changes

gateway/main/settings.py Show resolved Hide resolved

IceKhan13 reviewed Oct 23, 2023

View reviewed changes

client/quantum_serverless/core/job.py Outdated Show resolved Hide resolved

akihikokuroda force-pushed the worker2 branch from 7b0f714 to 43d1e8e Compare October 25, 2023 18:17

akihikokuroda requested review from IceKhan13 and psschwei October 25, 2023 20:40

psschwei reviewed Oct 26, 2023

View reviewed changes

IceKhan13 added the feature branch label Oct 26, 2023

IceKhan13 modified the milestone: 0.7 Oct 26, 2023

akihikokuroda marked this pull request as draft October 26, 2023 15:04

akihikokuroda marked this pull request as ready for review October 26, 2023 18:37

akihikokuroda force-pushed the worker2 branch from 4a08b4c to 2ea1df9 Compare October 29, 2023 13:01

akihikokuroda added 2 commits November 3, 2023 17:38

add workers config

5c8fecb

Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>

change ProgramConfig to JobConfig

bcc8ce1

Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>

akihikokuroda force-pushed the worker2 branch from 6e0dba7 to bcc8ce1 Compare November 3, 2023 21:40

fix merge error

a03ada2

akihikokuroda mentioned this pull request Nov 6, 2023

Add python_version to the Configuration #1077

Merged

IceKhan13 self-assigned this Nov 6, 2023

IceKhan13 reviewed Nov 6, 2023

View reviewed changes

IceKhan13 requested changes Nov 6, 2023

View reviewed changes

IceKhan13 removed their assignment Nov 6, 2023

review comments

6196936

add tests

a85c930

akihikokuroda requested review from psschwei and IceKhan13 November 8, 2023 17:27

IceKhan13 approved these changes Nov 8, 2023

View reviewed changes

IceKhan13 added the release candidate PR which is a candidate for next release label Nov 8, 2023

IceKhan13 changed the base branch from main to development November 10, 2023 19:53

IceKhan13 deleted the branch Qiskit:development November 13, 2023 14:51

IceKhan13 closed this Nov 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add worker config options #1051

Add worker config options #1051

akihikokuroda commented Oct 23, 2023

akihikokuroda commented Oct 23, 2023

psschwei left a comment

psschwei Oct 23, 2023

akihikokuroda Oct 23, 2023

akihikokuroda commented Oct 23, 2023

Tansito commented Oct 23, 2023 •

edited

Loading

akihikokuroda commented Oct 23, 2023

psschwei commented Oct 23, 2023

Tansito commented Oct 23, 2023

psschwei commented Oct 23, 2023

IceKhan13 Oct 23, 2023

akihikokuroda Oct 25, 2023

psschwei left a comment

psschwei Oct 26, 2023

akihikokuroda Oct 26, 2023

akihikokuroda commented Oct 26, 2023

psschwei commented Oct 26, 2023

Tansito commented Nov 3, 2023

IceKhan13 commented Nov 3, 2023

akihikokuroda commented Nov 3, 2023

IceKhan13 commented Nov 6, 2023

IceKhan13 Nov 6, 2023 •

edited

Loading

IceKhan13 Nov 6, 2023

IceKhan13 Nov 6, 2023

IceKhan13 left a comment

akihikokuroda commented Nov 6, 2023

IceKhan13 commented Nov 6, 2023

akihikokuroda commented Nov 7, 2023

akihikokuroda commented Nov 8, 2023

IceKhan13 left a comment

akihikokuroda commented Nov 8, 2023

IceKhan13 commented Nov 13, 2023 •

edited

Loading

Add worker config options #1051

Add worker config options #1051

Conversation

akihikokuroda commented Oct 23, 2023

Summary

Details and comments

akihikokuroda commented Oct 23, 2023

psschwei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akihikokuroda commented Oct 23, 2023

Tansito commented Oct 23, 2023 • edited Loading

akihikokuroda commented Oct 23, 2023

psschwei commented Oct 23, 2023

Tansito commented Oct 23, 2023

psschwei commented Oct 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psschwei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akihikokuroda commented Oct 26, 2023

psschwei commented Oct 26, 2023

Tansito commented Nov 3, 2023

IceKhan13 commented Nov 3, 2023

akihikokuroda commented Nov 3, 2023

IceKhan13 commented Nov 6, 2023

IceKhan13 Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IceKhan13 left a comment

Choose a reason for hiding this comment

akihikokuroda commented Nov 6, 2023

IceKhan13 commented Nov 6, 2023

akihikokuroda commented Nov 7, 2023

akihikokuroda commented Nov 8, 2023

IceKhan13 left a comment

Choose a reason for hiding this comment

akihikokuroda commented Nov 8, 2023

IceKhan13 commented Nov 13, 2023 • edited Loading

Tansito commented Oct 23, 2023 •

edited

Loading

IceKhan13 Nov 6, 2023 •

edited

Loading

IceKhan13 commented Nov 13, 2023 •

edited

Loading