Skip to content

add vmargs=-XX:-UseContainerSupport in config - #136

Merged
maaquib merged 4 commits into
masterfrom
issue_99
Mar 16, 2023
Merged

add vmargs=-XX:-UseContainerSupport in config#136
maaquib merged 4 commits into
masterfrom
issue_99

Conversation

@lxning

@lxning lxning commented Jan 14, 2023

Copy link
Copy Markdown
Contributor

Issue #, if available:
#99

Description of changes:
Apply the fixing in pytorch inference toolkit

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@davidthomas426

davidthomas426 commented Jan 22, 2023

Copy link
Copy Markdown

I'll just note that this issue along with workarounds and fixes have shown up across different inference toolkits.

Here is a list of links showing that it's been a recurring problem.

Also, this fix may create other problems, as turning off container support means that the JVM does not respect Docker container memory limits.

We should make sure to address this uniformly across the inference toolkits and deep-learning-containers, while allowing users to easily customize without needing onerous workarounds such as using derived deep-learning-container images or even forking toolkit.

Links:

I still don't think this is an exhaustive list.

@chen3933

chen3933 commented Feb 2, 2023

Copy link
Copy Markdown
Contributor

The PR will be updated to allow customization of vmargs
Example :
aws/sagemaker-inference-toolkit@c01dde7

@rohithkrn

Copy link
Copy Markdown

python 3.7 tests failing at coverage report step. Failing to invoke coverage command. Works fine for python3.6
ERROR: InvocationError for command /codebuild/output/src309395522/src/github.com/aws/sagemaker-pytorch-inference-toolkit/.tox/py37/bin/coverage report --fail-under=90 --include '*sagemaker_pytorch_serving_container*' (exited with code 1)

chen3933
chen3933 previously approved these changes Mar 6, 2023
@maaquib
maaquib requested a review from rohithkrn March 16, 2023 21:41
@maaquib maaquib mentioned this pull request Mar 16, 2023
Comment thread tox.ini Outdated
@maaquib maaquib self-assigned this Mar 16, 2023
@maaquib
maaquib merged commit cfb83d2 into master Mar 16, 2023
@maaquib
maaquib deleted the issue_99 branch March 16, 2023 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants