Implement config panel for swap management #1858
Conversation
|
Some annoying warnings appear in the logs, does anyone know how to mask them?
|
|
I add a swappiness configuration to avoid to write on ssd if not necessary (in order to extend its service life). I wonder if we should adapt the code in order to support do_pre_regen and be able to --dry-run or to detect this file is managed by yunohost. However, if someone change the swappiness value, i guess it's for good reasons and we don't need to remember the user that it's configured like that. |
| if [ -f /swap_file ] && [ $(stat -c%s /swap_file) -ne $swapfile_size ]; then | ||
| # Let's delete it first if it is not of the requested size | ||
| ynh_print_info --message="Deleting swap first..." | ||
| ynh_del_swap |
There was a problem hiding this comment.
That's really dangerous. Let's imagine this situation:
- RAM is 1GB, usage 900MB, Swapfile is 1GB, usage 500MB
- you want a 2GB swap file now, so we fall into this code
- ynh_del_swap deletes the original swap file, making the server (or services) most probably crash from OOM (1.4GB required on a 1GB available ram)
There was a problem hiding this comment.
Two better solutions would be :
- just create a second swap file of size = difference (in my example, 2GB(total) - 1GB(existing) = 1GB new swap file)
- create a second swap file of the total requested swap size, then afterwards delete the old file. The danger being the disk space but eh, i think we are okay, ram/swap is way smaller than disks usually…
There was a problem hiding this comment.
Ah good, thank you for giving some thoughts into this. :)
I had added a very lazy "Beware, if you reduce its current size, you may crash your server." in the config panel, but indeed there are other ways to crash everything.
I think your proposal is good!
| # | ||
| # usage: ynh_add_swap --size=SWAP in MiB | ||
| # | arg: -s, --size= - Amount of SWAP to add in MiB. | ||
| ynh_add_swap () { |
There was a problem hiding this comment.
We need an additional helper, that would "just" call yunohost settings {get,set} 'swap.swapfile.swapfile_size' and make new_swap_size = current_swap_size + requested_app_swap_size.
There was a problem hiding this comment.
yunohost settings set got me in an infinite config panel loop last night, we need to be careful with calling hooks from confif panels. ;)
|
All in all… I would prefer if swap file creation was NOT linked to apps requesting swap especially, but rather to the RAM usage declared in the apps manifest. |
|
I completely agree, that's why this is a system config panel, not a general apps config panel. |
Yeah, something like a button on the app installation page redirecting to the swap config panel you're adding. We already have a warning if the RAM usage reported by the manifest is too high. |
…--dir=the_dir_to_check
… sysctl 999-yunohost-swap.conf
Co-authored-by: ljf (zamentur) <[email protected]>
| ynh_handle_getopts_args "$@" | ||
|
|
||
| # Could be moved to an argument | ||
| swapfile_dir="$(realpath /)" |
There was a problem hiding this comment.
Uuuuuh, why would / be ... something else than / ? x_x
The problem
Swap management is non existent on YunoHost, and we historically let apps set their own, which has made many core dev very angry and has been widely regarded as a bad move.
Solution
PR Status
Tested, works for me. Please try :)
How to test
...