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

Fix for Validation Interval can be set more frequent than Backup Interval #2207

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

VandalByte
Copy link
Contributor

Description

This PR resolves the issue in the Schedule Tab where the Validate Interval QSpinBox allowed users to set the validation schedule to run more frequently than the backup schedule. This behavior was problematic since validation (using BorgCheckJob) only runs after a backup, meaning that it shouldn't be scheduled more frequently than the backup itself.

With this change, Vorta now ensures that the validation interval cannot be set to a value smaller than the backup interval, enforcing a more logical and consistent scheduling behavior.

Related Issue

Resolves #1971

Motivation and Context

  • This change is required to enforce logical consistency between the backup and validation intervals in the Vorta scheduling system.
  • Previously, users were able to set the validation interval to run more frequently than the backup interval, which could lead to issues, as validation (via BorgCheckJob) only occurs after a backup.

How Has This Been Tested?

  • Tested the UI in the Schedule Tab to ensure that the Validate Interval QSpinBox no longer allows validation intervals smaller than the backup interval.
  • Attempted to set a validation interval shorter than the backup interval, and confirmed that the UI prevents this configuration.

Discussion:

  • I've added a new function on_validation_change() to /src/vorta/views/schedule_page.py.
    • The function checks if Validation is enabled and if the Backup schedule is set to a periodic value with the unit 'week'.
    • Since validation is measured in weeks, other units should not interfere with this logic.
    • The function ensures that the validation interval cannot be set to a value lower than the backup interval.
  • On starting Vorta, the function is executed to set the validation interval value based on the backup interval.
  • I also modified the on_scheduler_change() function to ensure this validation runs when the scheduler count or unit is changed.
  • These changes prevent the user from setting the validation interval lower than the backup interval in the UI.

Types of changes

  •  Bug fix (non-breaking change which fixes an issue)
  •  New feature (non-breaking change which adds functionality)
  •  Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read the CONTRIBUTING guide.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

@VandalByte
Copy link
Contributor Author

I realized this issue affects other units as well.
For example, in periodic backup, if the interval is set to 12 days and validation is 1 week, the same issue occurs.

So, I added checks for all units and ensured that the minimum value restriction applies only to 'Backup periodically'

@m3nu
Copy link
Contributor

m3nu commented Feb 17, 2025

Previously, users were able to set the validation interval to run more frequently than the backup interval, which could lead to issues, as validation (via BorgCheckJob) only occurs after a backup.

What are the issues you talk about here?

@VandalByte
Copy link
Contributor Author

@m3nu
So I haven't covered the full repo yet, I just looked at the UI part, but I did some backtracking, and from what I understand if the time period is set like mentioned above, when BorgCheckJob.prepare() runs, it would likely fail as it can't find any backups and return msg['ok'] = False and this would be logged -> 'Conditions for backup not met. Aborting.'.

The issue is actually handled, and there shouldn't be any issues with normal functioning of Vorta, my PR adds the logical consistency at UI level. Please do correct me if I'm wrong somewhere.

@m3nu
Copy link
Contributor

m3nu commented Feb 17, 2025

I see 2 options here:

  1. Add the code you suggested to automatically adjust validation interval. We need to maintain this new code in the future and possibly adjust it if scheduling changes.
  2. Leave it as it is now. There may be confusion because validation doesn't run as often.

Not sure which one is better.

@VandalByte
Copy link
Contributor Author

@m3nu
So I tested it out a bit more to see which is the best option in this case. First, I tested the Borg command line version 1.2.8.

TEST PROCESS

I intialized a new borg repo with borg init --encryption=repokey ~/Videos/bbbb/. Then I ran a validation check on that repo with no backups inside. This was the output:

$ borg check ~/Videos/bbbb --verbose

Starting repository check
finished segment check at segment 1
Starting repository index check
Index object count match.
Finished full repository check, no problems found.
Starting archive consistency check...
Enter passphrase for key /home/vandal/Videos/bbbb: 
Archive consistency check complete, no problems found.

So it's properly handled in the borg's end.

In src/vorta/borg/check.py line 45, I found the borg command used by vorta for validation as:

cmd = ['borg', 'check', '--info', '--log-json', '--progress']

I ran a small python script to check/validate the output of borg repo without any backups again, to see if any issues popped up.

import subprocess
import os

repo_path = os.path.expanduser('~/Music/bbbb')
cmd = ['borg', 'check', '--info', '--log-json', '--progress', repo_path]

try:
    result = subprocess.run(cmd, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
    print("Borg check: SUCCESS")
    print(result.stdout)
except subprocess.CalledProcessError as e:
    print(f"Borg check: FAILED ==> {e}")
    print(e.stderr)

Output was Borg check: SUCCESS, borg process output was also same as above. So no matter if there's a backup or not the borg check will always be successful and not throw any errors.

Also, the process will be logged as:

{
  "type": "log_message",
  "time": 1740655354.3571105,
  "message": "Archive consistency check complete, no problems found.",
  "levelname": "INFO",
  "name": "borg.archive"
}

CONCLUSION

  • Validation command (borg check) can be run even without a backup present and it will not throw an error.
  • The changes introduced by this PR is not necessary for the proper functioning of Vorta.
  • The level of logical consistency brought by this PR is not worth all the additional lines of code.

Since it seems more logical to not include these changes in the final implementation, I suggest either closing or marking this PR as a draft for future discussion.

@m3nu
Copy link
Contributor

m3nu commented Feb 27, 2025

Thanks for testing! Good to know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation Interval should not be more frequent than Backup Interval
2 participants