Make docker_server nginx max payload size configurable - #2601
Draft
FredLiu876 wants to merge 1 commit into
Draft
Conversation
The in-container nginx reverse proxy generated for docker_server deployments hardcodes client_max_body_size to 64M, so custom server users hit a 413 at exactly 64 MiB with no way to raise it. Adds an optional docker_server.max_payload_size that feeds the nginx template, validated against nginx size syntax. Default behavior is unchanged.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The nginx reverse proxy Truss generates in front of a
docker_server(custom server) container hardcodesclient_max_body_sizeto64M:truss/base/constants.py:34—TRUSSLESS_MAX_PAYLOAD_SIZE = "64M"truss/contexts/image_builder/serving_image_builder.py:461— passed intoproxy.conf.jinjaCustom-server users get a
413at exactly 64 MiB with no way to raise it. It reads like a platform ingress limit (our docs say the ingress cap is 100 MB), but it is generated into the customer's own image, so nothing on their side or in their Truss config controls it.This came from a customer who bracketed it precisely: highest accepted body 66,837,864 B (63.74 MiB), lowest rejected 67,850,445 B (64.71 MiB).
64M= 67,108,864 B sits inside that bracket. A request trace confirmed every Baseten hop passed the full body through and the 413 was generated after dispatch into the model pod.Change
Adds an optional
docker_server.max_payload_size, validated against nginx size syntax (integer with optionalk/m/gsuffix), threaded into the nginx template. Falls back toTRUSSLESS_MAX_PAYLOAD_SIZEwhen unset.Default behavior is unchanged — a config without the field renders byte-identical nginx output.
Sizing caveat for reviewers
This is why the field is opt-in and the default is untouched.
proxy.conf.jinjasetsclient_body_buffer_size 4Mand pointsclient_body_temp_pathat/dev/shm, so any body over 4 MB spills into pod memory./dev/shmis an explicit memory-backedemptyDir(operatormodel.service.template.yaml.jinja:283-286, mounted at:906-907) and itssizeLimitscales with GPU count, not instance RAM (operator/core/utils/workload_utils.py:120-138):The tmpfs counts against the container memory limit —
actions.py:1687says so directly ("the tempfs usage is limited by the container memory limit"). So oversizing this gives you ENOSPC pastsizeLimit, or an OOMKill if shm growth blows the cgroup. The template also setserror_log /dev/null, so that failure is silent.That is the argument against simply raising
TRUSSLESS_MAX_PAYLOAD_SIZE. The constant is baked into everydocker_serverimage: on the 1 GiB tier, 128 MiB bodies cap out around 8 concurrent, and there are ~6,400 CPU-onlydocker_servermodels on1x2(1 GiB shm inside a 2 GiB cgroup) plus ~1,800 on single-GPU L4/A10G/T4. An opt-in field leaves all of them untouched.Open question for reviewers: should the validator enforce an upper bound instead of accepting
1G? A safer long-term fix may be movingclient_body_temp_pathoff/dev/shmonto a small disk-backedemptyDir— #2163 moved it to/dev/shmonly to dodge read-only filesystems, which a dedicated emptyDir also solves.Note this does not by itself raise the end-to-end ceiling: the WP ingress controller is
proxy-body-size: 100m, so 100 MiB is the most that is reachable over the public path today.Test plan
truss/tests/test_config.py::test_docker_server_invalid_max_payload_size— rejects64MBtruss/tests/contexts/image_builder/test_serving_image_builder.py::test_nginx_config_disables_disk_writes— parametrized: default renders64M, explicit128Mrenders128Muv run pytest truss/tests/test_config.py truss/tests/contexts/image_builder/test_serving_image_builder.py -m 'not integration'— 287 passed, 3 skipped.ruff check/ruff format --checkclean.bin/generate_truss_config_schema.py --checkup to date.Draft — opening for direction on the sizing questions above before marking ready.