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.

Sign up for GitHub

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

Jump to bottom

fix(restore): multiple restore requests should be rejected and proposals should not be submitted #7118

Merged
merged 5 commits into from
Dec 15, 2020
Merged

fix(restore): multiple restore requests should be rejected and proposals should not be submitted #7118

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7670909
fix(restore): multiple restore requests should be rejected and propos…
aman-bansal Dec 11, 2020
bde9060
adding doc defining restore behaviour
aman-bansal Dec 11, 2020
d76c0f0
changing message to be more concrete
aman-bansal Dec 11, 2020
240a23e
making restore return error if operation already running
aman-bansal Dec 13, 2020
ed63e7e
making use of GetOngoingTasks
aman-bansal Dec 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions wiki/content/enterprise-features/binary-backups.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,12 @@ leases are updated accordingly. The latest backup to be restored should contain
the same number of groups in its manifest.json file as the cluster to which it
is being restored.

{{% notice "note" %}}
When running a live restore, you only need to send the request to a single Alpha.
If you run the same restore on a different Alpha at the same time,
then another restore operation will be started.
{{% /notice %}}

Restore can be performed from Amazon S3 / Minio or from a local directory. Below
is the documentation for the fields inside `RestoreInput` that can be passed into
the mutation.
Expand Down
10 changes: 5 additions & 5 deletions worker/online_restore_ee.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,14 @@ func ProcessRestoreRequest(ctx context.Context, req *pb.RestoreRequest) (int, er
if err := VerifyBackup(req, &creds, currentGroups); err != nil {
return 0, errors.Wrapf(err, "failed to verify backup")
}

if err := FillRestoreCredentials(req.Location, req); err != nil {
return 0, errors.Wrapf(err, "cannot fill restore proposal with the right credentials")
}
restoreId, err := rt.Add()
if err != nil {
return 0, errors.Wrapf(err, "cannot assign ID to restore operation")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cannot assign ID to restore operation, because of?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrapf will take care of that. rt.Add() returns multiple kind of errors. wrapf will combine message to make it more meaningfull. This is the interface it implements. func (w *withMessage) Error() string { return w.msg + ": " + w.cause.Error() } Adding something in errors will mess up with the api response. But yes we can add some logging if needed. But seems redundant here.

}

req.RestoreTs = State.GetTimestamp(false)

// TODO: prevent partial restores when proposeRestoreOrSend only sends the restore
Expand All @@ -80,10 +84,6 @@ func ProcessRestoreRequest(ctx context.Context, req *pb.RestoreRequest) (int, er
}()
}

restoreId, err := rt.Add()
if err != nil {
return 0, errors.Wrapf(err, "cannot assign ID to restore operation")
}
go func(restoreId int) {
errs := make([]error, 0)
for range currentGroups {
Expand Down