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

call restic backup script with nice (wip) #111

Merged
merged 2 commits into from
Jan 26, 2024

Conversation

felixandersen
Copy link
Contributor

Opening this PR as I did this change locally and thought it would add value to the role. Possibly not as is, with a default 10 priority, but an optional priority parameter? If given could prefix command with /usr/bin/nice -n [priority]. Let me know if this is interesting and I can refine the change.

@DO1JLR
Copy link
Member

DO1JLR commented Jan 3, 2024

How did I not heard about nice before... 🤔

Sounds like a good idea. Please continue ❤️

@felixandersen
Copy link
Contributor Author

@DO1JLR I have now added a conditional part in the service-template as well as some explanation in the README. Let me know if anything needs to be changed.

@kdebisschop
Copy link

systemd has an option for Nice in the service

[Service]
    Type=simple
    Restart=always
    RestartSec=60
    #ExecStartPre=/bin/sleep 20 
    User=colibri
    Nice=-10
    StandardOutput=null
    StandardError=null

Seems like maybe it would be better to use that? Or maybe even add a dictionary of service options that could be added as a loop.

@kdebisschop
Copy link

kdebisschop commented Jan 25, 2024

The generic version of code is just to add

{% if item.service_options is defined %}
{% for key, value in item.service_options.items() %}
{{key}}={{value}}
{% endfor %}
{% endif %}

after Type=oneshot in restic.service.j2

Then in a job in restic_backups you can add e.g.,

    service_options:
      Nice: "-10"
      CPUQuota: "10%"

@felixandersen
Copy link
Contributor Author

@kdebisschop that is great input and I agree it would be a better option. One potential drawback of a full dictionary approach would be complexity where a user might break a config by specifying something unexpected. I see it as a net positive tradeoff though.

I vote for full dictionary. At the very least we should use the Nice option in the service instead of wrapping the call like I did.

I have not heard anything from @DO1JLR in a while, but I am happy to rework my PR as @DO1JLR see fit.

@DO1JLR
Copy link
Member

DO1JLR commented Jan 26, 2024

sorry

@DO1JLR DO1JLR enabled auto-merge January 26, 2024 14:44
@DO1JLR DO1JLR merged commit 9a1cafd into roles-ansible:main Jan 26, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants