Skip to content

Update configuration to improve performance and reliability #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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Feb 24, 2021
Merged

Update configuration to improve performance and reliability #9

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3d57c75
Set keepalive_timeout to 65s
itskingori Feb 22, 2021
c4a9530
Set client_header_timeout to 65s
itskingori Feb 22, 2021
b298dc5
Set client_max_body_size to 500m
itskingori Feb 22, 2021
4a8d7ef
Set send_timeout to 60s
itskingori Feb 22, 2021
2a4d30b
Set client_body_buffer_size to 128k
itskingori Feb 22, 2021
6e03bd6
Prefer to align values
itskingori Feb 22, 2021
f7d2a73
Improve logical grouping of configuration
itskingori Feb 22, 2021
0ab6f23
Bump version and update changelog
itskingori Feb 22, 2021
a58861c
Adjust client_header_timeout and keepalive_timeout to 605s
itskingori Feb 24, 2021
4134a44
Set proxy_connect_timeout to 60s
itskingori Feb 24, 2021
47db2e6
Set proxy_send_timeout to 60s
itskingori Feb 24, 2021
a56a0d2
Update changelog
itskingori Feb 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## 1.19.5-6

* Set `client_body_buffer_size` to `128k`.
* Set `client_header_timeout` to `65s`.
* Set `client_max_body_size` to `500m`.
* Set `keepalive_timeout` to `65s`.
* Set `send_timeout` to `60s`.

## 1.19.5-5

* Add `http_x_amzn_trace_id` to json logging (for `X-Amzn-Trace-Id` header).
Expand Down
19 changes: 11 additions & 8 deletions config/http.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,17 @@ http {
include /etc/nginx/mime.types;
include /etc/nginx/log.conf;

server_tokens off;
keepalive_timeout 20s;
sendfile on;
tcp_nopush on;
client_max_body_size 400m;
client_body_timeout 300s;
server_tokens off;
sendfile on;
tcp_nopush on;

client_max_body_size 500m;
Copy link
Member

@zacblazic zacblazic Feb 25, 2021

Choose a reason for hiding this comment

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

Any reason for increasing to 500m?

Copy link
Member Author

@itskingori itskingori Feb 25, 2021

Choose a reason for hiding this comment

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

Not real reason other than it feels like a nice round number 😅 ... 400m feels very random.

client_body_buffer_size 128k;
Copy link
Member

@zacblazic zacblazic Feb 25, 2021

Choose a reason for hiding this comment

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

What's the intention behind increasing this?

Copy link
Member Author

@itskingori itskingori Feb 25, 2021

Choose a reason for hiding this comment

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

There are some links in the commit for more context but the TL;DR is that it's to:

  1. Reduce the number of "a client request body is buffered to a temporary file" warning we get.
  2. Reduce the number of times nginx has to write file (search for above string in logs to see). Memory is faster. Also since we have client_max_body_size also figured uploads would benefit too by having to write even less.

It's not that great of an issue, more like a nice-to-have rather than must-have. FYI, you can see evidence of this below ...

Screenshot 2021-02-25 at 14 31 40

client_body_timeout 300s;
Copy link
Member

@zacblazic zacblazic Feb 25, 2021

Choose a reason for hiding this comment

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

Wondering if we should align the client_body_timeout value with the client_header_timeout?

Copy link
Member Author

@itskingori itskingori Feb 25, 2021

Choose a reason for hiding this comment

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

No objections to that. Should probably have.

client_header_timeout 65s;
keepalive_timeout 65s;
proxy_read_timeout 600s;
send_timeout 60s;
Copy link
Member

@zacblazic zacblazic Feb 25, 2021

Choose a reason for hiding this comment

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

Are we setting these to their defaults just for visibility?

Copy link
Member Author

@itskingori itskingori Feb 25, 2021

Choose a reason for hiding this comment

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

Yes. For visibility.


# If we receive X-Forwarded-Proto, pass it through; otherwise, pass along the
# scheme used to connect to this server
Expand Down Expand Up @@ -49,8 +54,6 @@ http {
proxy_set_header X-Request-ID $proxy_x_request_id;
proxy_set_header X-Forwarded-Host $host;

proxy_read_timeout 600s;

include /etc/nginx/app.conf;

server {
Expand Down
4 changes: 2 additions & 2 deletions config/main.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
worker_processes auto;
worker_rlimit_nofile 8192;
worker_processes auto;
worker_rlimit_nofile 8192;
worker_shutdown_timeout 630s;

events {
Expand Down