Skip to content

Performance updates to nginx configuration. #28

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Rider-Linden
Copy link
Contributor

No description provided.

@Rider-Linden Rider-Linden requested a review from Copilot June 30, 2025 22:34
Copilot

This comment was marked as outdated.

@Rider-Linden Rider-Linden requested a review from Copilot July 2, 2025 20:45
Copilot

This comment was marked as outdated.

Rider-Linden and others added 2 commits July 2, 2025 13:58
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Rider-Linden Rider-Linden changed the title adding keepalives to upstream connection Performance updates to nginx configuration. Jul 7, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances NGINX performance by introducing configurable file descriptor limits and separating WSGI timeouts from keepalive settings. It also sets sensible defaults in the entrypoint script and updates related documentation.

  • Add worker_rlimit_nofile controlled by WORKER_FILE_LIMIT.
  • Replace KEEPALIVE_TIMEOUT with WSGI_TIMEOUT for uWSGI timeouts.
  • Provide defaults for new env vars in 00-render-templates.sh and update docs.

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/etc/nginx/nginx.conf.template Added worker_rlimit_nofile {{ .Env.WORKER_FILE_LIMIT }}
src/etc/nginx/includes/uwsgi.conf.template Swapped uwsgi_read/send_timeout to use WSGI_TIMEOUT
src/docker-entrypoint.d/00-render-templates.sh Defaulted WORKER_FILE_LIMIT and WSGI_TIMEOUT env variables
README.md Documented WORKER_FILE_LIMIT; missing entry for new WSGI_TIMEOUT
Dockerfile Removed unnecessary blank line for consistent formatting
Comments suppressed due to low confidence (5)

src/etc/nginx/includes/uwsgi.conf.template:5

  • The new WSGI_TIMEOUT variable is not documented in README.md; please add an entry explaining its purpose and default behavior.
uwsgi_read_timeout {{ .Env.WSGI_TIMEOUT }};

README.md:31

  • [nitpick] Clarify that the default 2*WORKER_CONNECTIONS value is computed at runtime by the entrypoint script rather than set literally in the environment.
| `WORKER_FILE_LIMIT` | Set the number of available file descriptors | No | 2*WORKER_CONNECTIONS | 4096 |

src/etc/nginx/nginx.conf.template:10

  • Consider adding a test or CI check to verify that WORKER_FILE_LIMIT is rendered correctly into the final nginx.conf, ensuring the intended file descriptor limit is applied.
worker_rlimit_nofile {{ .Env.WORKER_FILE_LIMIT }};

src/docker-entrypoint.d/00-render-templates.sh:20

  • If KEEPALIVE_TIMEOUT is undefined, WSGI_TIMEOUT may be empty; consider providing a fallback or validating that KEEPALIVE_TIMEOUT is set before using it.
export WSGI_TIMEOUT=${WSGI_TIMEOUT:-${KEEPALIVE_TIMEOUT}}

@Rider-Linden Rider-Linden force-pushed the rider/config-tuning branch from 5f02ce1 to 24ff62d Compare July 10, 2025 23:24
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.

2 participants