backup: move retry loop to entire download phase

The retry loop introduced in #148821 specifically would retry the
`DownloadSpan` requests. Moving the retry loop to handle the entire
download phase would also allow us to retry the cleanup step as well.

Epic: CRDB-51394

Release note: None

Author: kev-cao

pkg/backup/restore_job.go

@@ -1793,7 +1793,7 @@ func (r *restoreResumer) doResume(ctx context.Context, execCtx interface{}) erro
 		if err := p.ExecCfg().JobRegistry.CheckPausepoint("restore.before_do_download_files"); err != nil {
 			return err
 		}
-		return r.doDownloadFiles(ctx, p)
+		return r.doDownloadFilesWithRetry(ctx, p)
 	}
 
 	if err := p.ExecCfg().JobRegistry.CheckPausepoint("restore.before_load_descriptors_from_backup"); err != nil {
@@ -2026,7 +2026,7 @@ func (r *restoreResumer) doResume(ctx context.Context, execCtx interface{}) erro
 		// TODO(msbutler): ideally doDownloadFiles would not depend on job details
 		// and is instead passed an execCfg and the download spans and anything else
 		// it needs. If that occured, we would not need to update details above.
-		if err := r.doDownloadFiles(ctx, p); err != nil {
+		if err := r.doDownloadFilesWithRetry(ctx, p); err != nil {
 			return err
 		}
 	}

pkg/backup/restore_online.go

@@ -563,28 +563,14 @@ func (r *restoreResumer) sendDownloadWorker(
 				return err
 			}
 
-			var err error
-			for r := retry.StartWithCtx(ctx, retry.Options{
-				InitialBackoff: time.Millisecond * 100,
-				MaxBackoff:     time.Second,
-				MaxRetries:     maxDownloadAttempts - 1,
-			}); r.Next(); {
-				err = func() error {
-					if testingKnobs != nil && testingKnobs.RunBeforeSendingDownloadSpan != nil {
-						if err := testingKnobs.RunBeforeSendingDownloadSpan(); err != nil {
-							return err
-						}
-					}
-					return sendDownloadSpan(ctx, execCtx, spans)
-				}()
-				if err == nil {
-					break
+			if testingKnobs != nil && testingKnobs.RunBeforeSendingDownloadSpan != nil {
+				if err := testingKnobs.RunBeforeSendingDownloadSpan(); err != nil {
+					return err
 				}
-				log.VInfof(ctx, 1, "attempt %d failed to download spans: %v", r.CurrentAttempt(), err)
 			}
 
-			if err != nil {
-				return errors.Wrapf(err, "retries exhausted for sending download spans")
+			if err := sendDownloadSpan(ctx, execCtx, spans); err != nil {
+				return err
 			}
 
 			// Wait for the completion poller to signal that it has checked our work.
@@ -818,6 +804,24 @@ func getRemainingExternalFileBytes(
 	return remaining, nil
 }
 
+func (r *restoreResumer) doDownloadFilesWithRetry(
+	ctx context.Context, execCtx sql.JobExecContext,
+) error {
+	var err error
+	for rt := retry.StartWithCtx(ctx, retry.Options{
+		InitialBackoff: time.Millisecond * 100,
+		MaxBackoff:     time.Second,
+		MaxRetries:     maxDownloadAttempts - 1,
+	}); rt.Next(); {
+		err = r.doDownloadFiles(ctx, execCtx)
+		if err == nil {
+			return nil
+		}
+		log.Warningf(ctx, "failed attempt #%d to download files: %v", rt.CurrentAttempt(), err)
+	}
+	return errors.Wrapf(err, "retries exhausted for downloading files")
+}
+
 func (r *restoreResumer) doDownloadFiles(ctx context.Context, execCtx sql.JobExecContext) error {
 	details := r.job.Details().(jobspb.RestoreDetails)