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

add systemd configs for astral indexers #389

Merged
merged 1 commit into from
Jan 20, 2025
Merged

Conversation

DaMandal0rian
Copy link
Member

@DaMandal0rian DaMandal0rian commented Jan 20, 2025

User description

add systemd configs for astral indexers


PR Type

enhancement, configuration changes


Description

  • Added systemd configuration for Astral Indexers service.

  • Introduced log rotation configuration for Astral Indexers logs.

  • Configured resource limits and environment variables for the service.

  • Ensured automatic service restart and log management.


Changes walkthrough 📝

Relevant files
Configuration changes
logrotate
Added log rotation configuration for Astral Indexers         

explorer/astral/logrotate

  • Added log rotation configuration for Astral Indexers logs.
  • Configured daily rotation with 7 backups and compression.
  • Included post-rotation service restart command.
  • +12/-0   
    systemd
    Added systemd configuration for Astral Indexers service   

    explorer/astral/systemd

  • Added systemd unit file for Astral Indexers service.
  • Configured environment variables and working directory.
  • Set resource limits for CPU and memory usage.
  • Enabled automatic restart and log redirection.
  • +23/-0   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @DaMandal0rian DaMandal0rian changed the title add systemd configs for explorer add systemd configs for astral indexers Jan 20, 2025
    @DaMandal0rian DaMandal0rian merged commit 213378e into main Jan 20, 2025
    1 check passed
    @DaMandal0rian DaMandal0rian deleted the astral-systemd branch January 20, 2025 11:02
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Log Rotation Postrotate Command

    The postrotate command in the logrotate configuration restarts the astral-indexers.service. This could potentially cause service interruptions. Consider whether this behavior is acceptable or if a more graceful log handling approach is needed.

    postrotate
        systemctl restart astral-indexers.service > /dev/null 2>/dev/null || true
    endscript
    Resource Limits Validation

    The CPUQuota and MemoryMax values in the systemd configuration should be validated to ensure they align with the expected resource usage and system capacity.

    CPUQuota=80%
    MemoryMax=52G

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Improve error handling during log rotation

    Ensure that the postrotate script checks the success of systemctl restart
    astral-indexers.service and logs any errors instead of suppressing them with >
    /dev/null 2>/dev/null || true, as this can hide critical issues during log rotation.

    explorer/astral/logrotate [9-11]

     postrotate
    -    systemctl restart astral-indexers.service > /dev/null 2>/dev/null || true
    +    if ! systemctl restart astral-indexers.service; then
    +        echo "Failed to restart astral-indexers.service" >> /var/log/astral-indexers.log
    +    fi
     endscript
    Suggestion importance[1-10]: 9

    Why: The suggestion improves error handling by ensuring that failures in restarting the service during log rotation are logged. This is crucial for debugging and maintaining system reliability, as the current implementation suppresses all errors.

    9
    General
    Switch to systemd journal for logging

    Use StandardOutput and StandardError with journal instead of appending to a log file
    to leverage systemd's built-in logging and avoid potential file size issues.

    explorer/astral/systemd [15-16]

    -StandardOutput=append:/var/log/astral-indexers.log
    -StandardError=append:/var/log/astral-indexers.log
    +StandardOutput=journal
    +StandardError=journal
    Suggestion importance[1-10]: 8

    Why: Switching to systemd's journal for logging simplifies log management and avoids potential file size issues. This is a significant improvement in terms of maintainability and leveraging systemd's built-in capabilities.

    8
    Set file descriptor limit in systemd

    Add a LimitNOFILE directive to the systemd service file to explicitly set a file
    descriptor limit, preventing potential issues with too many open files.

    explorer/astral/systemd [18-20]

     # Resource limits
     CPUQuota=80%
     MemoryMax=52G
    +LimitNOFILE=65536
    Suggestion importance[1-10]: 7

    Why: Adding a LimitNOFILE directive is a proactive measure to prevent potential issues with too many open files. While not critical, it enhances system stability and resource management.

    7

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Status: Done
    Development

    Successfully merging this pull request may close these issues.

    1 participant