Create a new multiprocessing context instead of using default context to avoid RuntimeError: context has already been set #3407

sivanantha321 · 2024-02-03T10:42:41Z

What this PR does / why we need it:
Currently, we are using default multiprocessing context to set the process start method to fork, since FastApi/Uvicorn server does not support spawn or forkserver. If the end user already sets this start method we are getting RuntimeError: context has already been set error. This PR creates a new multiprocessing context and uses that to set the process start method. So, modifying the default context using multiprocessing.set_start_method function no longer affects our model server.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #3406

Type of changes
Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Feature/Issue validation/testing:

Please describe the tests that you ran to verify your changes and relevant result summary. Provide instructions so it can be reproduced.
Please also list any relevant details for your test configuration.

Test A
Test B
Logs

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Checklist:

Have you added unit/e2e tests that prove your fix is effective or that this feature works?
Has code been commented, particularly in hard-to-understand areas?
Have you made corresponding changes to the documentation?

Release note:

Create a new multiprocessing context instead of using default context to avoid RuntimeError: context has already been set

terrytangyuan

/lgtm

rachitchauhan43 · 2024-02-03T18:26:28Z

python/kserve/kserve/model_server.py

+                if start_method is None:
+                    logger.info("Setting 'fork' as multiprocessing start method.")
+                    multiprocessing.set_start_method('fork')
+                elif start_method in ['spawn', 'forkserver']:


Can we add inline comment on why we only support 'fork' and not the other 2 ? Rest LGTM

here the workers are only meant for uvicorn processes, if it is intended to launch a model with multi-processing then it would not work, for example pytorch does not support fork.
https://pytorch.org/docs/stable/notes/multiprocessing.html#cuda-in-multiprocessing

Can we add inline comment on why we only support 'fork' and not the other 2 ? Rest LGTM

There is already a comment above the changed lines. Because of the way github shows changes, you might have missed it. But looking into this PR will give you more context though :)

oss-prow-bot · 2024-02-04T15:27:11Z

New changes are detected. LGTM label has been removed.

sivanantha321 · 2024-02-04T15:43:35Z

python/kserve/kserve/model_server.py

                self._rest_server = UvicornServer(self.http_port, [serversocket],
                                                  self.dataplane, self.model_repository_extension,
                                                  self.enable_docs_url, log_config=self.log_config,
                                                  access_log_format=self.access_log_format)
+                # Since py38 MacOS/Windows defaults to use spawn for starting multiprocessing.
+                # https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods
+                # Spawn does not work with FastAPI/uvicorn in multiprocessing mode, use fork for multiprocessing


Interestingly, uvicorn internally uses 'spawn' for creating new subprocesses.

oss-prow-bot · 2024-02-04T20:44:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rachitchauhan43, sivanantha321, terrytangyuan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~python/kserve/OWNERS~~ [rachitchauhan43]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

yuzisun · 2024-02-09T10:42:51Z

python/kserve/kserve/model_server.py

+                # https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods
+                # Spawn does not work with FastAPI/uvicorn in multiprocessing mode, use fork for multiprocessing
+                # https://github.com/tiangolo/fastapi/issues/1586
+                context = multiprocessing.get_context('fork')


but this breaks the development on Mac?

serdarildercaglar · 2024-03-25T12:34:45Z

Note: Tried after merging sivanantha321:fix-multiprocess-context into the master on local

When I run the server with multiple workers, It starts properly.

But when I send the request to the server, the model gets the payload but is stuck in the prediction step.

But during the inference time, the server gets stuck and can't return a prediction. After I wait nearly 20 minutes, I try to stop the server combination of ctrl+c.

Then, When I want to run server again, I get error.

@sivanantha321

kjoth · 2024-04-03T14:55:49Z

Hope this is fixed and new version is released. Facing this issue when trying to set --workers in docker .

oss-prow-bot bot requested review from lizzzcai and sukumargaonkar February 3, 2024 10:42

terrytangyuan approved these changes Feb 3, 2024

View reviewed changes

oss-prow-bot bot assigned terrytangyuan Feb 3, 2024

oss-prow-bot bot added the lgtm label Feb 3, 2024

rachitchauhan43 reviewed Feb 3, 2024

View reviewed changes

sivanantha321 force-pushed the fix-multiprocess-context branch from 8278bbb to 4f336ea Compare February 4, 2024 15:27

oss-prow-bot bot removed the lgtm label Feb 4, 2024

sivanantha321 commented Feb 4, 2024

View reviewed changes

rachitchauhan43 approved these changes Feb 4, 2024

View reviewed changes

oss-prow-bot bot added the approved label Feb 4, 2024

sivanantha321 changed the title ~~Only set multiprocessing start method if it is not already set~~ Create a new multiprocessing context instead of using default context Feb 5, 2024

sivanantha321 changed the title ~~Create a new multiprocessing context instead of using default context~~ Create a new multiprocessing context instead of using default context to avoid RuntimeError: context has already been set Feb 5, 2024

sivanantha321 force-pushed the fix-multiprocess-context branch from 4f336ea to 6348f55 Compare February 7, 2024 05:54

sivanantha321 marked this pull request as draft February 7, 2024 06:27

oss-prow-bot bot added the do-not-merge/work-in-progress label Feb 7, 2024

sivanantha321 added 2 commits February 8, 2024 19:04

Only set multiprocessing start method if it is not already set

d11e140

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

Create a new multiprocess context to set start method

70483d0

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

sivanantha321 force-pushed the fix-multiprocess-context branch from 6348f55 to 70483d0 Compare February 8, 2024 13:36

sivanantha321 marked this pull request as ready for review February 9, 2024 05:29

oss-prow-bot bot removed the do-not-merge/work-in-progress label Feb 9, 2024

oss-prow-bot bot requested review from cmaddalozzo and njhill February 9, 2024 05:29

yuzisun reviewed Feb 9, 2024

View reviewed changes

sivanantha321 marked this pull request as draft February 14, 2024 08:50

oss-prow-bot bot added the do-not-merge/work-in-progress label Feb 14, 2024

sivanantha321 mentioned this pull request Mar 25, 2024

Model server throws error when multiprocessing start method is already set #3406

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a new multiprocessing context instead of using default context to avoid RuntimeError: context has already been set #3407

Create a new multiprocessing context instead of using default context to avoid RuntimeError: context has already been set #3407

sivanantha321 commented Feb 3, 2024 •

edited

terrytangyuan left a comment

rachitchauhan43 Feb 3, 2024 •

edited

yuzisun Feb 3, 2024

sivanantha321 Feb 4, 2024

oss-prow-bot bot commented Feb 4, 2024

sivanantha321 Feb 4, 2024

oss-prow-bot bot commented Feb 4, 2024

yuzisun Feb 9, 2024

serdarildercaglar commented Mar 25, 2024 •

edited

kjoth commented Apr 3, 2024

Create a new multiprocessing context instead of using default context to avoid RuntimeError: context has already been set #3407

Are you sure you want to change the base?

Create a new multiprocessing context instead of using default context to avoid RuntimeError: context has already been set #3407

Conversation

sivanantha321 commented Feb 3, 2024 • edited

terrytangyuan left a comment

Choose a reason for hiding this comment

rachitchauhan43 Feb 3, 2024 • edited

Choose a reason for hiding this comment

yuzisun Feb 3, 2024

Choose a reason for hiding this comment

sivanantha321 Feb 4, 2024

Choose a reason for hiding this comment

oss-prow-bot bot commented Feb 4, 2024

sivanantha321 Feb 4, 2024

Choose a reason for hiding this comment

oss-prow-bot bot commented Feb 4, 2024

yuzisun Feb 9, 2024

Choose a reason for hiding this comment

serdarildercaglar commented Mar 25, 2024 • edited

kjoth commented Apr 3, 2024

sivanantha321 commented Feb 3, 2024 •

edited

rachitchauhan43 Feb 3, 2024 •

edited

serdarildercaglar commented Mar 25, 2024 •

edited