Address duplicate timestamps in metadata mirror files #328
Conversation
FIX: fail with meaningful error message on metadata mirror files with duplicate timestamps, closes #322 NEW: option --allow-duplicate-timestamps to only warn about duplicate timestamps in metadata mirror files, use this option with care and only to clean an impacted backup repository.
|
Look good. |
|
I'll do all necessary changes in this one PR (that's why it's still WIP). |
ericzolf
changed the title
|
Not any more WIP: test case has been written and works for me. Please review and merge. |
|
You just need top press the button next to the note that it's not up to date. |
| # Normally there shouldn't be any case of duplicate timestamp but it seems | ||
| # we had the issue at some point in time, hence we need the flag to allow | ||
| # users to clean up their repository. The default is to abort on such cases. | ||
|
|
There was a problem hiding this comment.
Actually, users don't need this flag to clean up their repository with '--remove-older-than'. The code path for 'remove-older-than' never calls sorted_prefix_inclist(). I therefore suggest that this option flag be removed because it serves no other useful purpose, and because its existence may be confusing to users, and because users may use it inappropriately which may cause additional problems for them.
There was a problem hiding this comment.
Well spotted, I'm impressed, and I was almost ready to remove the option, when I realized that the --check-destination-dir does fail in such cases, which raises new "philosophical" questions. Let us continue this discussion in the new PR, once I've created it (I will definitely change the man pages).
| extra_options=b"--allow-duplicate-timestamps") | ||
| # now we can clean-up | ||
| rdiff_backup(1, 1, target_rp.get_path(), None, | ||
| extra_options=b"--allow-duplicate-timestamps --remove-older-than 5B --force") |
There was a problem hiding this comment.
As said before, the --allow-duplicate-timestamps option is not needed here.
|
I can't re-open a merged PR, but you're feedback is correct and I'll address it in a new PR. Thanks. |
- re-write the tests to take into consideration that --remove-older-than doesn't need --allow-duplicate-timestamps - improve the error message and the man-page also accordingly - make the tests also slightly more robust, sorting entry and using date instead of number of backups
- re-write the tests to take into consideration that --remove-older-than doesn't need --allow-duplicate-timestamps - improve the error message and the man-page also accordingly - make the tests also slightly more robust, sorting entry and using date instead of number of backups
FIX: fail with meaningful error message on metadata mirror files with duplicate timestamps, closes #322
NEW: option --allow-duplicate-timestamps to only warn about duplicate timestamps in metadata mirror files, use this option with care and only to clean an impacted backup repository.
I still need to add a test for this and document the new option in the manpage and in the completion files.