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

do rotation only, if maximum previous level exists [patch] #18

Closed
Marwe opened this issue Oct 8, 2013 · 4 comments
Closed

do rotation only, if maximum previous level exists [patch] #18

Marwe opened this issue Oct 8, 2013 · 4 comments
Milestone

Comments

@Marwe
Copy link

Marwe commented Oct 8, 2013

I created a feature request a whila ago on
http://sourceforge.net/p/rsnapshot/feature-requests/30/

Since sourceforge seems to be dead or at least taken over by criminals [1], I repeat it here and add the patch file.
Hi,
here I have a simple patch that prevents rotation in a level, if there is nothing to roll into it from the lower level (e.g. no daily.{max} -> do not rotate weekly to prevent disappearing weekly.0).
Until now rotation is always done, and leads to holes in the backup scheme (e.g. weekly.[013] exist,weekly.[2] missing due to forced rotation).
This can then later lead to holes in the next levels (e.g. monthly.0 missing, if weekly.3 missing during monthly rotation), too.

I regard this as bug and the attached patch is quite simple.
Somebody should check and decide,
a) if there should be an option for the old behaviour (the 'dumb'/forced rotate).
b) if there are any problems with just bailing out then (I dont think so, but there are some options checked in the following parts).

Thanks for looking into it,
Martin

[1] http://www.gluster.org/2013/08/how-far-the-once-mighty-sourceforge-has-fallen/

@Marwe
Copy link
Author

Marwe commented Oct 8, 2013

Hm, I don't know where to add a patch file, git just works via forks I guess.
Whatever, patch is on sourceforge (see link above), a patch against a more recent git version pasted here:

diff --git a/rsnapshot-program.pl b/rsnapshot-program.pl
index 5fea412..b8e5d81 100755
--- a/rsnapshot-program.pl
+++ b/rsnapshot-program.pl
@@ -4226,7 +4226,15 @@ sub rotate_higher_interval {
    my $interval_max        = $$id_ref{'interval_max'};
    my $prev_interval       = $$id_ref{'prev_interval'};
    my $prev_interval_max   = $$id_ref{'prev_interval_max'};
-   
+
+   # here we check, if $prev_interval.$prev_interval_max exists and only do the rotation, if from that level can be pulled...
+   # otherwise bail. Maybe there should be an option for such new behaviour, too?
+   if (! -d "$config_vars{'snapshot_root'}/$prev_interval.$prev_interval_max" ) {
+       print_err ("Did not find previous interval max ($config_vars{'snapshot_root'}/$prev_interval.$prev_interval_max), refusing to rotate this level ($interval)", 1);
+       syslog_err("Did not find previous interval max ($config_vars{'snapshot_root'}/$prev_interval.$prev_interval_max), refusing to rotate this level ($interval)");
+       bail();
+   }
+
    # ROTATE DIRECTORIES
    #
    # delete the oldest one (if we're keeping more than one)

myrdd pushed a commit to myrdd/rsnapshot that referenced this issue Jun 23, 2014
With this bugfix, gaps like a missing <interval>.0 folder will be prevented.
See bug rsnapshot#18.
@myrdd
Copy link
Contributor

myrdd commented Jun 23, 2014

Thanks for your patch! I've created a pull request (#34). There I've added some code to the rotate_lowest_snapshots() subroutine, which prevents rotation of the highest interval if sync_first is enabled but the .sync directory doesn't exist.

@bebehei bebehei added this to the rsnapshot 1.4 milestone Mar 27, 2015
@bebehei bebehei modified the milestones: rsnapshot 1.5, rsnapshot 1.4 Jun 15, 2015
bebehei pushed a commit to bebehei/rsnapshot that referenced this issue Jun 18, 2015
With this bugfix, gaps like a missing <interval>.0 folder will be prevented.
See bug rsnapshot#18.
@nkadel-skyhook
Copy link

Looks like the code got merged. Close this one?

@Marwe
Copy link
Author

Marwe commented May 25, 2016

Thanks for the great work, further discussions, additions and merges. closing.

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

No branches or pull requests

4 participants