[RFC] dir: Clean caches when free space is below threshold #1690
Conversation
If the pull failed due to lack of free space error, it is not recommended to hold onto partial caches (in repo/tmp) which can leave the disk-space full. Low-level free disk space error reporing is not reliable so we alternatively check for free space using fstatfs(). Hence, we keep partial caches only if free space is above a certain threshold. flatpak#1119
|
Can one of the admins verify this patch?
|
|
bot, test pull request |
| if (g_error_matches (my_error, G_IO_ERROR, G_FILE_ERROR_NOSPC) || | ||
| flatpak_dir_check_for_free_space (repo)) | ||
| { | ||
| flatpak_dir_cache_clean (repo); |
There was a problem hiding this comment.
This feels like something that should live in libostree by default; deleting everything in the repo's tmp directory is also going to break anything else concurrently using that directory (although in practice other things would probably also fail with ENOSPC too).
There was a problem hiding this comment.
Living in ostree would probably also make it easier to correctly propagate the ENOSPC error.
There was a problem hiding this comment.
I wonder here. The logic here is that, the default action is to clean up (in case of failure), unless the user has free-space above a certain threshold. For e.g. keep caches only if free space is above 5GBs.
The primary reason I chose this is to work around the ENOSPC error (and it's problem in error-reporting).
There was a problem hiding this comment.
Hi, made a related PR to this in ostree. Please comment on it. ostreedev/ostree#1602
| file = g_file_new_for_path (glnx_fdrel_abspath (dfd, "tmp")); | ||
|
|
||
| flatpak_rm_rf (file, NULL, NULL); | ||
| } |
There was a problem hiding this comment.
I'm thinking, in the pull case, we should only remove our staging directory, not everything.
In a generic, we're low on disk case we could delete everything, but we should take the repo lock in exclusive mode to avoid running parallel operations mysteriously failing.
There was a problem hiding this comment.
@alexlarsson Should I take a repo lock and remove the repo/tmp in case of low-disk? That means the latter case you mentioned above.
Also, for the former case, I am not sure how will I get the path of the staging-* in order to remove it.
|
☔ The latest upstream changes (presumably ac2d51b) made this pull request unmergeable. Please resolve the merge conflicts. |
|
This is now being implemented in Ostree: ostreedev/ostree#1602 |
If the pull failed due to lack of free space error, it is
not recommended to hold onto partial caches (in repo/tmp) which
can leave the disk-space full. Low-level free disk space error
reporing is not reliable so we alternatively check for free space
using fstatfs(). Hence, we keep partial caches only if free space is
above a certain threshold.
#1119