Skip to content

Commit ae83b4d

Browse files
committed
backup: fix download job progress tracker ctx awareness
Currently, `waitForDownloadToComplete` of the download job implicitly relies on calls to `getExternalBytes` to detect when the context has cancelled so that it can error accordingly. We should do this explicitly. The retry mechanism actually does support context awareness and calls to `retry.Next` will return `false` if the context is canceled, but we currently discard the results of `retry.Next`. This commit updates the tracker to properly break on a `false`. Epic: None Release note: None
1 parent c994ab7 commit ae83b4d

File tree

1 file changed

+3
-2
lines changed

1 file changed

+3
-2
lines changed

pkg/backup/restore_online.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ func (r *restoreResumer) waitForDownloadToComplete(
713713
var lastProgressUpdate time.Time
714714
for rt := retry.StartWithCtx(
715715
ctx, retry.Options{InitialBackoff: time.Second, MaxBackoff: time.Second * 10},
716-
); ; rt.Next() {
716+
); rt.Next(); {
717717
remaining, err := getExternalBytesOverSpans(ctx, execCtx.ExecCfg(), details.DownloadSpans)
718718
if err != nil {
719719
return errors.Wrap(err, "failed to get remaining external file bytes")
@@ -752,6 +752,7 @@ func (r *restoreResumer) waitForDownloadToComplete(
752752
default:
753753
}
754754
}
755+
return ctx.Err()
755756
}
756757

757758
func unstickRestoreSpans(
@@ -981,7 +982,7 @@ func (r *restoreResumer) maybeCleanupFailedOnlineRestore(
981982
ctx context.Context, p sql.JobExecContext, details jobspb.RestoreDetails,
982983
) error {
983984
if len(details.DownloadSpans) == 0 {
984-
// If this job is completly unrelated OR, exit early.
985+
// If this job is completely unrelated to OR, exit early.
985986
return nil
986987
}
987988

0 commit comments

Comments
 (0)