Merge pull request #9306 from dolthub/aaron/clone-robustness-under-remote-conjoin

go: store/datas/pull: clone.go: Improve robust of Clone for certain remoteapi implementations when the remote Conjoins.

Author: reltuk

go/libraries/doltcore/remotestorage/chunk_store.go

@@ -36,6 +36,7 @@ import (
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/trace"
 	"golang.org/x/sync/errgroup"
+	"google.golang.org/protobuf/types/known/timestamppb"
 
 	remotesapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/remotesapi/v1alpha1"
 	"github.com/dolthub/dolt/go/libraries/doltcore/remotestorage/internal/reliable"
@@ -1228,6 +1229,17 @@ func (drtf DoltRemoteTableFile) Open(ctx context.Context) (io.ReadCloser, uint64
 	}
 
 	if resp.StatusCode/100 != 2 {
+		// XXX: If we fail to fetch, and we have the ability
+		// to refresh the URL, set ourselves up to refresh the
+		// next time we are used. This can help if the
+		// remoteapi endpoint gave us a URL that actually
+		// expires for some reason before the RefreshAfter
+		// timestamp. (Local clock drift, remote key rotation,
+		// etc.)
+		if drtf.info.RefreshAfter != nil {
+			drtf.info.RefreshAfter = timestamppb.Now()
+			drtf.info.RefreshAfter.Seconds -= 10
+		}
 		defer resp.Body.Close()
 		body := make([]byte, 4096)
 		n, _ := io.ReadFull(resp.Body, body)

go/store/datas/pull/clone.go

@@ -204,11 +204,34 @@ func clone(ctx context.Context, srcTS, sinkTS chunks.TableFileStore, sinkCS chun
 		if failureCount >= maxAttempts {
 			return err
 		}
-		if _, sourceFiles, appendixFiles, err = srcTS.Sources(ctx); err != nil {
+		if _, refreshedSourceFiles, refreshedAppendixFiles, err := srcTS.Sources(ctx); err != nil {
 			return err
 		} else {
-			tblFiles = filterAppendicesFromSourceFiles(appendixFiles, sourceFiles)
-			_, fileIDToTF, _ = mapTableFiles(tblFiles)
+			refreshedTblFiles := filterAppendicesFromSourceFiles(refreshedAppendixFiles, refreshedSourceFiles)
+			_, refreshedFileIDToTF, _ := mapTableFiles(refreshedTblFiles)
+			// Sources() will refresh remote table file
+			// sources with new download URLs. However, it
+			// will only return URLs for table files which
+			// are in the remote manifest, which could
+			// have changed since the clone started. Here
+			// we keep around any old TableFile instances
+			// for any TableFiles which have been
+			// conjoined away or have been the victim of a
+			// garbage collection run on the remote.
+			//
+			// If these files are no longer accessible,
+			// for example because the URLs expired
+			// without a RefreshTableFileUrlRequest being
+			// provided, or because the table files
+			// themselves have been removed from storage,
+			// then continuing to use these sources will
+			// fail terminally eventually. But in the
+			// case of doltremoteapi on DoltHub, using
+			// these Sources() will continue to work and
+			// will allow the Clone to proceed.
+			for k, v := range refreshedFileIDToTF {
+				fileIDToTF[k] = v
+			}
 		}
 	}