Skip to content

Conversation

@rohithkrn
Copy link
Contributor

@rohithkrn rohithkrn commented Mar 31, 2023

Description of changes:
sagemaker batch transform sets SAGEMAKER_MAX_PAYLOAD_IN_MB env var which is currently not handled and passed to the model server. Therefore, model server sets deafult max_request_size ignoring the value set by user. This change fixes that.

Testing done:

  • Unit test added
  • Tested the change locally by installing modified toolkit in 763104351884.dkr.ecr.us-east-1.amazonaws.com/mxnet-inference:1.9.0-cpu-py38-ubuntu20.04-sagemaker container and passing SAGEMAKER_MAX_PAYLOAD_IN_MB variable to the container and see the corresponding max request size in model serving logs.

General

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have used the regional endpoint when creating S3 and/or STS clients (if appropriate)
  • I have updated any necessary documentation, including READMEs

Tests

  • [x ] I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have checked that my tests are not configured for a specific region or account (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

assert env.management_http_port == "1738"
assert env.safe_port_range == "1111-2222"
assert "-XX:-UseContainerSupport" in env.vmargs
assert env.max_request_size == 10485760
Copy link

Choose a reason for hiding this comment

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

it is better to use 10 10241024 instead of 10485760

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree, updated.

@sagemaker-bot
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: sagemaker-inference-toolkit-pr
  • Commit ID: cf28325
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

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.

4 participants