Add logic for --fallback-enabled and

... --allow-partly-done options

Author: boris-ilijic

pbm/restore/physical.go

@@ -249,31 +249,72 @@ func (r *PhysRestore) close(noerr, cleanup bool) {
 	if err != nil {
 		r.log.Warning("waiting for cluster status during cleanup: %v", err)
 	}
-	if cStatus == defs.StatusError && cleanup {
-		r.log.Warning("apply db data from %s", fallbackDir)
-		err := r.migrateFromFallbackDirToDBDir()
-		if err != nil {
-			r.log.Error("migrate from fallback dir: %v", err)
-		}
-	} else if (cStatus == defs.StatusDone || cStatus == defs.StatusPartlyDone) && cleanup {
-		r.log.Debug("clean-up dbpath")
-		err := removeAll(r.dbpath, r.log, getInternalLogFileSkipRule())
+	if cleanup {
+		r.resolveCleanupStrategy(cStatus)()
+	}
+
+	if r.fallback {
+		// free space by just deleting fallback dir in any other case
+		err = r.removeFallback()
 		if err != nil {
-			r.log.Error("flush dbpath %s: %v", r.dbpath, err)
+			r.log.Error("flush fallback: %v", err)
 		}
 	}
 
-	// free space by just deleting fallback dir in any other case
-	err = r.removeFallback()
+	if r.stopHB != nil {
+		close(r.stopHB)
+	}
+}
+
+// doFullCleanup is node's cleanup strategy for deleting the whole dbpath.
+func (r *PhysRestore) doFullCleanup() {
+	r.log.Debug("clean-up dbpath")
+	err := removeAll(r.dbpath, r.log, getInternalLogFileSkipRule())
 	if err != nil {
-		r.log.Error("flush fallback: %v", err)
+		r.log.Error("flush dbpath %s: %v", r.dbpath, err)
 	}
+}
 
-	if r.stopHB != nil {
-		close(r.stopHB)
+// doFallbackCleanup is node's cleanup strategy for recovering dbpath
+// from fallbacksync dir.
+func (r *PhysRestore) doFallbackCleanup() {
+	r.log.Warning("apply db data from %s", fallbackDir)
+	err := r.migrateFromFallbackDirToDBDir()
+	if err != nil {
+		r.log.Error("migrate from fallback dir: %v", err)
 	}
 }
 
+// resolveCleanupStrategy is helper which returns cleanup strategy based on
+// config parameters and cluster status.
+func (r *PhysRestore) resolveCleanupStrategy(clusterStatus defs.Status) func() {
+	if !r.fallback {
+		// fallback strategy is disabled
+		return r.doFullCleanup
+	}
+
+	// fallback strategy is enabled
+	switch clusterStatus {
+	case defs.StatusError:
+		// restore failed, fallback recovery will be applied
+		return r.doFallbackCleanup
+	case defs.StatusPartlyDone:
+		if r.allowPartlyDone {
+			// cluster is partly-done with new restore, and this node will be cleaned
+			return r.doFullCleanup
+		} else {
+			// cluster is partly-done, but anyway fallback recovery will be applied
+			return r.doFallbackCleanup
+		}
+	case defs.StatusDone:
+		// this shouldn't happen, because at least this node failed
+		// anyway, we'll cleanup this node
+		return r.doFullCleanup
+	}
+
+	return r.doFullCleanup
+}
+
 // waitClusterStatus blocks until cluster status file is set on one of final statuses.
 // It also checks HB to see if cluster is stucked.
 func (r *PhysRestore) waitClusterStatus() (defs.Status, error) {
@@ -364,9 +405,14 @@ func (r *PhysRestore) flush(ctx context.Context) error {
 		}
 	}
 
-	err = r.migrateDBDirToFallbackDir()
-	if err != nil {
-		return errors.Wrapf(err, "move files to fallback path")
+	if r.fallback {
+		err = r.migrateDBDirToFallbackDir()
+		if err != nil {
+			return errors.Wrapf(err, "move files to fallback path")
+		}
+	} else {
+		// fallback strategy is disabled, just wipe-up everything in dbpath
+		err = removeAll(r.dbpath, r.log)
 	}
 
 	return nil

pbm/restore/physical_test.go

@@ -6,10 +6,12 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/percona/percona-backup-mongodb/pbm/defs"
 	"github.com/percona/percona-backup-mongodb/pbm/log"
 )
 
@@ -221,6 +223,136 @@ func TestWaitMgoFreePort(t *testing.T) {
 	})
 }
 
+func TestResolveCleanupStrategy(t *testing.T) {
+	cmpStrategy := func(fn any) uintptr {
+		return reflect.ValueOf(fn).Pointer()
+	}
+	type strategyDesc string
+	const (
+		fallbackStrategy    strategyDesc = "fallback"
+		fullCleanupStrategy strategyDesc = "fullCleanup"
+	)
+
+	testCases := []struct {
+		desc            string
+		fallback        bool
+		allowPartlyDone bool
+		clusterStatus   defs.Status
+		wantStrategy    strategyDesc
+	}{
+		// fallback enabled
+		{
+			desc:            "fallback: enabled, allow-partly-done: enabled, status: error",
+			fallback:        true,
+			allowPartlyDone: true,
+			clusterStatus:   defs.StatusError,
+			wantStrategy:    fallbackStrategy,
+		},
+		{
+			desc:            "fallback: enabled, allow-partly-done: enabled, status: partly-done",
+			fallback:        true,
+			allowPartlyDone: true,
+			clusterStatus:   defs.StatusPartlyDone,
+			wantStrategy:    fullCleanupStrategy,
+		},
+		{
+			desc:            "fallback: enabled, allow-partly-done: enabled, status: done",
+			fallback:        true,
+			allowPartlyDone: true,
+			clusterStatus:   defs.StatusDone,
+			wantStrategy:    fullCleanupStrategy,
+		},
+
+		{
+			desc:            "fallback: enabled, allow-partly-done: disabled, status: error",
+			fallback:        true,
+			allowPartlyDone: false,
+			clusterStatus:   defs.StatusError,
+			wantStrategy:    fallbackStrategy,
+		},
+		{
+			desc:            "fallback: enabled, allow-partly-done: disabled, status: partly-done",
+			fallback:        true,
+			allowPartlyDone: false,
+			clusterStatus:   defs.StatusPartlyDone,
+			wantStrategy:    fallbackStrategy,
+		},
+		{
+			desc:            "fallback: enabled, allow-partly-done: disabled, status: done",
+			fallback:        true,
+			allowPartlyDone: false,
+			clusterStatus:   defs.StatusDone,
+			wantStrategy:    fullCleanupStrategy,
+		},
+
+		// fallback disabled
+		{
+			desc:            "fallback: disabled, allow-partly-done: enabled, status: error",
+			fallback:        false,
+			allowPartlyDone: true,
+			clusterStatus:   defs.StatusError,
+			wantStrategy:    fullCleanupStrategy,
+		},
+		{
+			desc:            "fallback: disabled, allow-partly-done: enabled, status: partly-done",
+			fallback:        false,
+			allowPartlyDone: true,
+			clusterStatus:   defs.StatusPartlyDone,
+			wantStrategy:    fullCleanupStrategy,
+		},
+		{
+			desc:            "fallback: disabled, allow-partly-done: enabled, status: done",
+			fallback:        false,
+			allowPartlyDone: true,
+			clusterStatus:   defs.StatusDone,
+			wantStrategy:    fullCleanupStrategy,
+		},
+
+		{
+			desc:            "fallback: disabled, allow-partly-done: disabled, status: error",
+			fallback:        false,
+			allowPartlyDone: false,
+			clusterStatus:   defs.StatusError,
+			wantStrategy:    fullCleanupStrategy,
+		},
+		{
+			desc:            "fallback: disabled, allow-partly-done: disabled, status: partly-done",
+			fallback:        false,
+			allowPartlyDone: false,
+			clusterStatus:   defs.StatusPartlyDone,
+			wantStrategy:    fullCleanupStrategy,
+		},
+		{
+			desc:            "fallback: disabled, allow-partly-done: disabled, status: done",
+			fallback:        false,
+			allowPartlyDone: false,
+			clusterStatus:   defs.StatusDone,
+			wantStrategy:    fullCleanupStrategy,
+		},
+	}
+
+	for _, tC := range testCases {
+		t.Run(tC.desc, func(t *testing.T) {
+			r := &PhysRestore{
+				fallback:        tC.fallback,
+				allowPartlyDone: tC.allowPartlyDone,
+				log:             log.DiscardLogger.NewDefaultEvent(),
+			}
+			strategy := r.resolveCleanupStrategy(tC.clusterStatus)
+			if tC.wantStrategy == fullCleanupStrategy {
+				if cmpStrategy(strategy) != cmpStrategy(r.doFullCleanup) {
+					t.Fatalf("want=%s", fullCleanupStrategy)
+
+				}
+			} else if tC.wantStrategy == fallbackStrategy {
+				if cmpStrategy(strategy) != cmpStrategy(r.doFallbackCleanup) {
+					t.Fatalf("want=%s", fallbackStrategy)
+				}
+			}
+		})
+	}
+}
+
 func TestRemoveAll(t *testing.T) {
 	t.Run("removes all files in dir", func(t *testing.T) {
 		tmpDir := setupTestFiles(t)