Skip to content
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 #440

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Nginx #440

wants to merge 9 commits into from

Conversation

wangxiaoming9168
Copy link

No description provided.

@what-the-diff
Copy link

what-the-diff bot commented Mar 30, 2023

PR Summary

  • Switch from Apache2 to Nginx
    Transition the web server from Apache2 to the more efficient Nginx.
  • Add Nginx configuration
    Introduce new configurations for Nginx, including timeout settings and keepalive_timeout.
  • Improve PostgreSQL performance
    Increase the shared_buffers value from 2GB to 16GB in postgres-tuning.conf for better database performance.
  • Update startup script
    Remove the old startapache script and introduce a new startnginx script for the Docker container's startup command (/app/startnginx).

Copy link
Collaborator

@philipkozeny philipkozeny left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Here's a quick review without testing.

@@ -146,4 +146,4 @@ EXPOSE 8080

COPY conf.d/env $PROJECT_DIR/.env

CMD ["/app/start.sh"]
CMD ["/app/start.sh"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

make sure that there is always a newline at the end of the file

4.2/cmd Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

this file can be removed altogether

Comment on lines +7 to +16
Timeout 600
StartServers 5
MinSpareServers 5
MaxSpareServers 10
MaxClients 150
MaxRequestsPerChild 0
HostnameLookups Off
KeepAlive On
MaxKeepAliveRequests 100
KeepAliveTimeout 15
Copy link
Collaborator

Choose a reason for hiding this comment

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

if we don't use apache anymore, we don't need the apache.conf altogether

@@ -1,10 +1,10 @@
# See https://nominatim.org/release-docs/4.2.0/admin/Installation/#tuning-the-postgresql-database
shared_buffers = 2GB
shared_buffers = 16GB
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason for that change?

maintenance_work_mem = 10GB
autovacuum_work_mem = 2GB
work_mem = 50MB
effective_cache_size = 24GB
synchronous_commit = off
max_wal_size = 1GB
checkpoint_timeout = 10min
checkpoint_completion_target = 0.9
checkpoint_completion_target = 0.9
Copy link
Collaborator

Choose a reason for hiding this comment

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

newline

@what-the-diff
Copy link

what-the-diff bot commented Mar 31, 2023

PR Summary

  • Switch from Apache to Nginx
    Change the web server from Apache to Nginx for better performance and efficiency
  • Add startnginx.sh script
    Introduce a new script to help with starting Nginx more easily during deployment
  • Update configuration files
    Replace the Apache configuration files with new Nginx configuration files to support the new web server
  • Update PHP handling in Dockerfile
    Remove the old PHP packages specific to Apache and introduce PHP 8 FPM for improved PHP processing with Nginx

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