-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nginx.conf: Use equal values for client_body_buffer_size and client_m… #28
Conversation
nginx.conf
Outdated
@@ -43,6 +43,10 @@ http { | |||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; # Add to x-forward-for | |||
proxy_read_timeout 900; | |||
proxy_buffering off; | |||
# These two should be the same or nginx will start writing | |||
# large request bodies to temp files | |||
client_body_buffer_size 10m; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want 10m? Is that the default buffer size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From http://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size looks like default values are 1m and 16k - having a bit larger sounds like a good idea right? But I guess the important part, judging from the Medium post, is that they're the same? rather than the value itself
(playing by ear here)
I'm concerned that if we enlarge the default, we'll consume all available
memory if too many users push. We know it currently works, but we don't
know exactly what might happen if we enlarge it 10x.
…On Fri, Dec 8, 2017 at 2:13 PM, Pablo Carranza Vélez < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In nginx.conf
<#28 (comment)>
:
> @@ -43,6 +43,10 @@ http {
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; # Add to x-forward-for
proxy_read_timeout 900;
proxy_buffering off;
+ # These two should be the same or nginx will start writing
+ # large request bodies to temp files
+ client_body_buffer_size 10m;
From http://nginx.org/en/docs/http/ngx_http_core_module.html#
client_max_body_size looks like default values are 1m and 16k - having a
bit larger sounds like a good idea right? But I guess the important part,
judging from the Medium post, is that they're the same? rather than the
value itself
(playing by ear here)
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#28 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA9GWV-RRYVx4heuo5F0qZnJrWaJvhHFks5s-bRvgaJpZM4Q7tUK>
.
|
That's a good point. I'll put both in 1m, that is the largest of the two defaults. |
Connects-to: https://github.com/resin-io/hq/issues/1115 |
…ax_body_size to avoid writing big files to disk Idea taken from https://medium.com/urban-massage-product/nginx-with-docker-easier-said-than-done-d1b5815d00d0 after noticing a user pushing a big layer causing out of space errors writing to /var/cache/nginx. Change-Type: patch Signed-off-by: Pablo Carranza Velez <[email protected]>
10cf649
to
4159eb5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great find @pcarranzav !
VersionBot failed to commit a new version to prepare a merge for the above pull request here: #28. The reason for this is: |
@pcarranzav, status checks have failed for this PR. Please make appropriate changes and recommit. |
I think it should help with #8 as well |
…ax_body_size to avoid writing big files to disk
Idea taken from https://medium.com/urban-massage-product/nginx-with-docker-easier-said-than-done-d1b5815d00d0 after noticing a user pushing a big layer
causing out of space errors writing to /var/cache/nginx.
Change-Type: patch
Signed-off-by: Pablo Carranza Velez [email protected]
Contributor checklist
Reviewer Guidelines