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

optimize squids #321

Merged
merged 2 commits into from
Jul 11, 2024
Merged

optimize squids #321

merged 2 commits into from
Jul 11, 2024

Conversation

DaMandal0rian
Copy link
Contributor

@DaMandal0rian DaMandal0rian commented Jul 10, 2024

User description

  • nginx improvements
  • change instance type

PR Type

enhancement


Description

  • Updated the default instance type in Terraform configuration from m6a.xlarge to m7a.2xlarge.
  • Optimized Nginx configuration by:
    • Setting worker_processes to auto.
    • Increasing worker_connections to 16000 and enabling multi_accept and epoll.
    • Adjusting various timeout and buffer settings for better performance.

Changes walkthrough 📝

Relevant files
Enhancement
variables.tf
Update default instance type to `m7a.2xlarge`                       

templates/terraform/explorer/base/variables.tf

  • Changed the default instance type from m6a.xlarge to m7a.2xlarge.
  • +1/-1     
    install_nginx.sh
    Optimize Nginx configuration for better performance           

    templates/terraform/explorer/base/scripts/install_nginx.sh

  • Set worker_processes to auto.
  • Increased worker_connections to 16000 and enabled multi_accept and
    epoll.
  • Adjusted various timeout and buffer settings for better performance.
  • +8/-6     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @github-actions github-actions bot added the enhancement New feature or request label Jul 10, 2024
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Configuration Validation:
    Ensure that the new Nginx settings (worker_processes auto, worker_connections 16000, multi_accept on, use epoll) are compatible and optimal for the server's hardware and workload.

    Performance Impact:
    The increase in keepalive_requests from 5000 to 200000 could have significant implications on server performance and resource usage. This should be tested under load to ensure it does not negatively impact the server.

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add checks for nginx and certbot installation before modifying configuration

    It's recommended to validate the presence of nginx and certbot before attempting to
    overwrite the nginx configuration to prevent script failure in case these are not
    installed.

    templates/terraform/explorer/base/scripts/install_nginx.sh [3]

    +if ! command -v nginx &> /dev/null || ! command -v certbot &> /dev/null
    +then
    +    echo "nginx or certbot not installed, exiting..."
    +    exit 1
    +fi
     cat /dev/null > /etc/nginx/nginx.conf
     
    Suggestion importance[1-10]: 10

    Why: Validating the presence of nginx and certbot before modifying the configuration prevents potential script failures, ensuring robustness.

    10
    Maintainability
    Replace hard-coded instance type with a variable

    Consider using a variable for the instance type default value to make it easier to
    update in the future and to maintain consistency across different environments or
    modules.

    templates/terraform/explorer/base/variables.tf [2]

    -default = "m7a.2xlarge"
    +default = var.default_instance_type
     
    Suggestion importance[1-10]: 8

    Why: Using a variable for the instance type default value improves maintainability and consistency across different environments or modules.

    8
    Performance
    Reduce the keepalive_requests value to prevent resource exhaustion

    Consider reducing the keepalive_requests value to a more typical range (e.g., 10000)
    to avoid potential resource exhaustion on high-traffic servers.

    templates/terraform/explorer/base/scripts/install_nginx.sh [28]

    -keepalive_requests 200000;
    +keepalive_requests 10000;
     
    Suggestion importance[1-10]: 7

    Why: Reducing the keepalive_requests value can help prevent resource exhaustion on high-traffic servers, enhancing performance and stability.

    7
    Increase the client body buffer size to handle larger requests more efficiently

    Increase client_body_buffer_size to a higher value such as 256K to better handle
    larger client requests and reduce the risk of buffer overflow.

    templates/terraform/explorer/base/scripts/install_nginx.sh [45]

    -client_body_buffer_size 128K;
    +client_body_buffer_size 256K;
     
    Suggestion importance[1-10]: 6

    Why: Increasing the client_body_buffer_size can help handle larger client requests more efficiently, but the suggested value should be carefully considered based on actual server requirements.

    6

    - nginx improvements
    - change instance type
    Copy link

    @marc-aurele-besner marc-aurele-besner left a comment

    Choose a reason for hiding this comment

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

    LGTM 🚀

    @DaMandal0rian DaMandal0rian merged commit 0678bc9 into main Jul 11, 2024
    1 check passed
    @DaMandal0rian DaMandal0rian deleted the squid-settings branch July 11, 2024 00:05
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants