search

YunoHost-Apps / synapse_ynh

Matrix server (synapse) package for YunoHost
https://matrix.org/
GNU General Public License v3.0
79 stars 42 forks source link

Borg backup failed to backup synapse application #474

Open Thatoo opened 5 months ago

Thatoo commented 5 months ago

Describe the bug

Borg fails when file changes on disk during archive creation and since I last updated synapse, it makes borg backup failing its backup. An issue has been open on the borg backup app repository but they say that the "maintainers of ynh_synapse could help with". https://github.com/YunoHost-Apps/borg_ynh/issues/159

Context

Steps to reproduce

2024-06-03 10:18:09,837: DEBUG - [2024-06-03_10:17:50] apps/synapse/backup/var/log/matrix-synapse/homeserver.log: file changed while we backed it up
2024-06-03 10:18:10,838: ERROR - Could not run script: /etc/yunohost/hooks.d/backup_method/05-borg_app
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/yunohost/hook.py", line 298, in hook_callback
    hook_return = hook_exec(
  File "/usr/lib/python3/dist-packages/yunohost/hook.py", line 416, in hook_exec
    raise YunohostError("hook_exec_failed", path=path)
yunohost.utils.error.YunohostError: Could not run script: /etc/yunohost/hooks.d/backup_method/05-borg_app

Expected behavior

Borg backup should be successful as it use to be before one of last synapse update.

Josue-T commented 5 months ago

Hello,

So to me there 2 issue linked to this thing.

Fist of all, you ran the backup while the service was running and it's really not recommended, mainly for data integrity reason. When you run the backup this way some data could change while the backup was made and so you will end with some data which depends on other which changed while the backup was running. Note that I's a known issue and there are already some issue about this somewhere else.

Over this previous described issue, we can have the question of should we backup the log in the apps. Until now on the apps that I maintained, the logs was I saved and restored (on backup/restore). But since https://github.com/YunoHost/package_linter/pull/154, we can suspect that the strategy should change.

To me the main issue about this is that normally we should recommend a way to manage the log for all apps the same way. So to me there are 2 possibility:

Until https://github.com/YunoHost/package_linter/pull/154 to me it was clearly the second one. But since this we have an unclear situation where we request that package don't remove the log but we still need to backup/restore following the example cf https://github.com/YunoHost/example_ynh/blob/master/scripts/backup#L64 which is a bit inconsistent.

Following this situation to me, firstly we must clarify how the log should be managed by all packages and following the decision of this, I'll adapt the package following the packaging guideline.

dmaroulidis commented 5 months ago

Hi @Josue-T,

Thanks for informing us about the log situation upstream. Until this is implemented, we could stop the service before backup following the advice the backup mechanism gives. The issue here is that Borg backup doesn't do that automatically and I don't see a way that this can be done automatically by the yunohost backup infrastructure.

How can we solve that? I think this needs to happen independently of how we manage logs, for the data integrity reasons you provided.

Josue-T commented 5 months ago

Hi,

Some more details about the data integrity issue.

To me it's a long time issue. To me ideally for all apps we should stop all services related to the app all time the backup is running. But some people would like to run the backup without stopping the service for availability reason. So there are no ideal choice which will make everybody happy with a decision.

About the implementation, we can't mange it in the package because the big data are saved after the end of the backup script. If the backup are run manually I recommend to stop the service manually before the backup and start it again when the backup are done. Note that on packages that I maintain you will have a warning if the service still run while you run the backup.

Ideally in long term this question should be solved by the core as it's same for all apps.

For automatic backup it's an other story, I case of borg I don't really know how it work exactly but to me this app should provide the possibility (as an option) to backup with stopping the service.

Well, for the log question, to me, it's strange that we have this issue only for this app. The data integrity is not problem for logs. But to me the problem is more about to have a standard way to fix this. Probably some other apps keep the log file open when the service is running and so the backup can't be done correctly. Or maybe the app don't backup the log but in this case is it the recommended way.

So mainly the response of how to solve that is that firstly we need to clarify the question and take consistent decision about all of theses thing before changing something in the package. Otherwise in the next 6 months we will need again to change something because something will break again.

utzer commented 4 months ago

Maybe the synapse log is just big compared to other logs, so it takes some time to backup, even when the backup is running every 2 hours? Maybe the changes are more frequent in the log file?

Who prepares the backup configuration for Yunohost for the matrix synapse application? I mean the standard backup configuration, does that stop the service before the backup, I think that config is used by borg. Maybe @Thatoo can explain this?

Thatoo commented 4 months ago

I don't know. What I can say is that yesterday evening I stop synapse service before my weekly borg backup and it has succeeded.

fflorent commented 4 months ago

This answer is very interesting: https://github.com/borgbackup/borg/issues/6989#issuecomment-1223921392

Quoting:

borg 1.2.x does not exit with an error (rc 2), but with a warning (rc 1) status. Basically it means you have to check the logs and decide for yourself whether it was something severe or not.

Also there is a retry implemented in borg 1.2: https://github.com/borgbackup/borg/pull/7351/commits/7e6afc93e918bc5549e8bd4d4153d1a47bc32566

So with my understanding, borg_ynh should check the borg create return code and if it is equal to 1 just log a warning instead of halting the script with an error.

utzer commented 2 months ago

Or just exclude that log path for synapse. 🤷‍♂️