From 265fdd16733e9d4da9da679b906e10447911f40a Mon Sep 17 00:00:00 2001 From: garethgeorge Date: Sat, 2 Dec 2023 12:53:53 -0800 Subject: [PATCH] fix: forget deadlocking and misc smaller bugs --- internal/config/environment.go | 2 +- internal/orchestrator/backup.go | 6 +++--- internal/orchestrator/forget.go | 25 ++++++++++++++++--------- internal/orchestrator/repo.go | 12 +++++++++--- pkg/restic/restic.go | 10 ---------- webui/src/lib/formatting.ts | 2 +- 6 files changed, 30 insertions(+), 27 deletions(-) diff --git a/internal/config/environment.go b/internal/config/environment.go index bf8c2c30..86b7a191 100644 --- a/internal/config/environment.go +++ b/internal/config/environment.go @@ -41,7 +41,7 @@ func BindAddress() string { } return val } - return "127.0.0.1:9898" + return ":9898" } func ResticBinPath() string { diff --git a/internal/orchestrator/backup.go b/internal/orchestrator/backup.go index 8d45bb7d..a8af27ec 100644 --- a/internal/orchestrator/backup.go +++ b/internal/orchestrator/backup.go @@ -148,11 +148,11 @@ func backupHelper(ctx context.Context, orchestrator *Orchestrator, plan *v1.Plan return fmt.Errorf("backup operation: %w", err) } + at := time.Now() if plan.Retention != nil { - orchestrator.ScheduleTask(NewOneofForgetTask(orchestrator, plan, op.SnapshotId, time.Now()), taskPriorityForget) + orchestrator.ScheduleTask(NewOneofForgetTask(orchestrator, plan, op.SnapshotId, at), taskPriorityForget) } - - orchestrator.ScheduleTask(NewOneofIndexSnapshotsTask(orchestrator, plan, time.Now()), taskPriorityIndexSnapshots) + orchestrator.ScheduleTask(NewOneofIndexSnapshotsTask(orchestrator, plan, at), taskPriorityIndexSnapshots) return nil } diff --git a/internal/orchestrator/forget.go b/internal/orchestrator/forget.go index 67107f97..e2e5d952 100644 --- a/internal/orchestrator/forget.go +++ b/internal/orchestrator/forget.go @@ -80,21 +80,28 @@ func (t *ForgetTask) Run(ctx context.Context) error { forgetOp.OperationForget.Forget = append(forgetOp.OperationForget.Forget, forgot...) + var ops []*v1.Operation for _, forgot := range forgot { if e := t.orchestrator.OpLog.ForEachBySnapshotId(forgot.Id, indexutil.CollectAll(), func(op *v1.Operation) error { - if indexOp, ok := op.Op.(*v1.Operation_OperationIndexSnapshot); ok { - indexOp.OperationIndexSnapshot.Forgot = true - if err := t.orchestrator.OpLog.Update(op); err != nil { - return fmt.Errorf("mark index snapshot %v as forgotten: %w", op.Id, err) - } - } - - // Soft delete the operation (can be recovered if necessary, todo: implement recovery). - return t.orchestrator.OpLog.Delete(op.Id) + ops = append(ops, op) + return nil }); e != nil { err = multierror.Append(err, fmt.Errorf("cleanup snapshot %v: %w", forgot.Id, e)) } } + for _, op := range ops { + if indexOp, ok := op.Op.(*v1.Operation_OperationIndexSnapshot); ok { + indexOp.OperationIndexSnapshot.Forgot = true + if e := t.orchestrator.OpLog.Update(op); err != nil { + err = multierror.Append(err, fmt.Errorf("mark index snapshot %v as forgotten: %w", op.Id, e)) + continue + } + } + // Soft delete the operation (can be recovered if necessary, todo: implement recovery). + if e := t.orchestrator.OpLog.Delete(op.Id); err != nil { + err = multierror.Append(err, fmt.Errorf("delete operation %v: %w", op.Id, e)) + } + } return err }); err != nil { diff --git a/internal/orchestrator/repo.go b/internal/orchestrator/repo.go index 57f878ff..dd107b1b 100644 --- a/internal/orchestrator/repo.go +++ b/internal/orchestrator/repo.go @@ -115,15 +115,21 @@ func (r *RepoOrchestrator) Forget(ctx context.Context, plan *v1.Plan) ([]*v1.Res l := zap.L().With(zap.String("repo", r.repoConfig.Id), zap.String("plan", plan.Id)) l.Debug("Forget snapshots", zap.Any("policy", policy)) - result, err := r.repo.Forget(ctx, protoutil.RetentionPolicyFromProto(plan.Retention), restic.WithFlags("--tag", tagForPlan(plan), "--group-by", "")) + result, err := r.repo.Forget( + ctx, protoutil.RetentionPolicyFromProto(plan.Retention), + restic.WithFlags("--tag", tagForPlan(plan)), restic.WithFlags("--group-by", "tag")) if err != nil { return nil, fmt.Errorf("get snapshots for repo %v: %w", r.repoConfig.Id, err) } - l.Debug("Forget result", zap.Int("forgot", len(result.Remove)), zap.Int("keep", len(result.Keep))) + l.Debug("Forget result", zap.Any("result", result)) var forgotten []*v1.ResticSnapshot for _, snapshot := range result.Remove { - forgotten = append(forgotten, protoutil.SnapshotToProto(&snapshot)) + snapshotProto := protoutil.SnapshotToProto(&snapshot) + if err := protoutil.ValidateSnapshot(snapshotProto); err != nil { + return nil, fmt.Errorf("snapshot validation failed: %w", err) + } + forgotten = append(forgotten, snapshotProto) } return forgotten, nil diff --git a/pkg/restic/restic.go b/pkg/restic/restic.go index b841b343..9cf31215 100644 --- a/pkg/restic/restic.go +++ b/pkg/restic/restic.go @@ -214,16 +214,6 @@ func (r *Repo) Forget(ctx context.Context, policy *RetentionPolicy, opts ...Gene return nil, NewCmdError(cmd, output, fmt.Errorf("invalid forget result: %w", err)) } - // then run the prune command - args = []string{"prune", "--json"} - args = append(args, r.extraArgs...) - args = append(args, opt.extraArgs...) - args = append(args, policy.toPruneFlags()...) - - cmd = exec.CommandContext(ctx, r.cmd, args...) - cmd.Env = append(cmd.Env, r.buildEnv()...) - cmd.Env = append(cmd.Env, opt.extraEnv...) - return &result[0], nil } diff --git a/webui/src/lib/formatting.ts b/webui/src/lib/formatting.ts index d285e307..f96e62a0 100644 --- a/webui/src/lib/formatting.ts +++ b/webui/src/lib/formatting.ts @@ -48,7 +48,7 @@ export const formatDate = (time: number | string | Date) => { }; export const formatDuration = (ms: number) => { - const seconds = Math.ceil(ms / 100); + const seconds = Math.ceil(ms / 1000); const minutes = Math.floor(seconds / 60); const hours = Math.floor(minutes / 60); if (hours === 0 && minutes === 0) {