Merge branch 'main' into lunny/small_opt

custom/conf/app.example.ini

@@ -2642,9 +2642,15 @@ LEVEL = Info
 ;; override the azure blob base path if storage type is azureblob
 ;AZURE_BLOB_BASE_PATH = lfs/
 
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;; settings for Gitea's LFS client (eg: mirroring an upstream lfs endpoint)
+;;
 ;[lfs_client]
-;; When mirroring an upstream lfs endpoint, limit the number of pointers in each batch request to this number
+;; Limit the number of pointers in each batch request to this number
 ;BATCH_SIZE = 20
+;; Limit the number of concurrent upload/download operations within a batch
+;BATCH_OPERATION_CONCURRENCY = 3
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

go.mod

@@ -124,6 +124,7 @@ require (
 	golang.org/x/image v0.21.0
 	golang.org/x/net v0.30.0
 	golang.org/x/oauth2 v0.23.0
+	golang.org/x/sync v0.8.0
 	golang.org/x/sys v0.26.0
 	golang.org/x/text v0.19.0
 	golang.org/x/tools v0.26.0
@@ -316,7 +317,6 @@ require (
 	go.uber.org/zap v1.27.0 // indirect
 	golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c // indirect
 	golang.org/x/mod v0.21.0 // indirect
-	golang.org/x/sync v0.8.0 // indirect
 	golang.org/x/time v0.7.0 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20241021214115-324edc3d5d38 // indirect
 	gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect

modules/gitrepo/gitrepo.go

@@ -11,6 +11,7 @@ import (
 
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 )
 
 type Repository interface {
@@ -59,15 +60,11 @@ func repositoryFromContext(ctx context.Context, repo Repository) *git.Repository
 	return nil
 }
 
-type nopCloser func()
-
-func (nopCloser) Close() error { return nil }
-
 // RepositoryFromContextOrOpen attempts to get the repository from the context or just opens it
 func RepositoryFromContextOrOpen(ctx context.Context, repo Repository) (*git.Repository, io.Closer, error) {
 	gitRepo := repositoryFromContext(ctx, repo)
 	if gitRepo != nil {
-		return gitRepo, nopCloser(nil), nil
+		return gitRepo, util.NopCloser{}, nil
 	}
 
 	gitRepo, err := OpenRepository(ctx, repo)
@@ -95,7 +92,7 @@ func repositoryFromContextPath(ctx context.Context, path string) *git.Repository
 func RepositoryFromContextOrOpenPath(ctx context.Context, path string) (*git.Repository, io.Closer, error) {
 	gitRepo := repositoryFromContextPath(ctx, path)
 	if gitRepo != nil {
-		return gitRepo, nopCloser(nil), nil
+		return gitRepo, util.NopCloser{}, nil
 	}
 
 	gitRepo, err := git.OpenRepository(ctx, path)

modules/lfs/http_client.go

@@ -17,6 +17,8 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/proxy"
 	"code.gitea.io/gitea/modules/setting"
+
+	"golang.org/x/sync/errgroup"
 )
 
 // HTTPClient is used to communicate with the LFS server
@@ -113,6 +115,7 @@ func (c *HTTPClient) Upload(ctx context.Context, objects []Pointer, callback Upl
 	return c.performOperation(ctx, objects, nil, callback)
 }
 
+// performOperation takes a slice of LFS object pointers, batches them, and performs the upload/download operations concurrently in each batch
 func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc DownloadCallback, uc UploadCallback) error {
 	if len(objects) == 0 {
 		return nil
@@ -133,71 +136,87 @@ func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc
 		return fmt.Errorf("TransferAdapter not found: %s", result.Transfer)
 	}
 
+	errGroup, groupCtx := errgroup.WithContext(ctx)
+	errGroup.SetLimit(setting.LFSClient.BatchOperationConcurrency)
 	for _, object := range result.Objects {
-		if object.Error != nil {
-			log.Trace("Error on object %v: %v", object.Pointer, object.Error)
-			if uc != nil {
-				if _, err := uc(object.Pointer, object.Error); err != nil {
-					return err
-				}
-			} else {
-				if err := dc(object.Pointer, nil, object.Error); err != nil {
-					return err
-				}
-			}
-			continue
-		}
+		errGroup.Go(func() error {
+			return performSingleOperation(groupCtx, object, dc, uc, transferAdapter)
+		})
+	}
 
-		if uc != nil {
-			if len(object.Actions) == 0 {
-				log.Trace("%v already present on server", object.Pointer)
-				continue
-			}
+	// only the first error is returned, preserving legacy behavior before concurrency
+	return errGroup.Wait()
+}
 
-			link, ok := object.Actions["upload"]
-			if !ok {
-				log.Debug("%+v", object)
-				return errors.New("missing action 'upload'")
-			}
+// performSingleOperation performs an LFS upload or download operation on a single object
+func performSingleOperation(ctx context.Context, object *ObjectResponse, dc DownloadCallback, uc UploadCallback, transferAdapter TransferAdapter) error {
+	// the response from a lfs batch api request for this specific object id contained an error
+	if object.Error != nil {
+		log.Trace("Error on object %v: %v", object.Pointer, object.Error)
 
-			content, err := uc(object.Pointer, nil)
-			if err != nil {
+		// this was an 'upload' request inside the batch request
+		if uc != nil {
+			if _, err := uc(object.Pointer, object.Error); err != nil {
 				return err
 			}
-
-			err = transferAdapter.Upload(ctx, link, object.Pointer, content)
-			if err != nil {
+		} else {
+			// this was NOT an 'upload' request inside the batch request, meaning it must be a 'download' request
+			if err := dc(object.Pointer, nil, object.Error); err != nil {
 				return err
 			}
+		}
+		// if the callback returns no err, then the error could be ignored, and the operations should continue
+		return nil
+	}
 
-			link, ok = object.Actions["verify"]
-			if ok {
-				if err := transferAdapter.Verify(ctx, link, object.Pointer); err != nil {
-					return err
-				}
-			}
-		} else {
-			link, ok := object.Actions["download"]
-			if !ok {
-				// no actions block in response, try legacy response schema
-				link, ok = object.Links["download"]
-			}
-			if !ok {
-				log.Debug("%+v", object)
-				return errors.New("missing action 'download'")
-			}
+	// the response from an lfs batch api request contained necessary upload/download fields to act upon
+	if uc != nil {
+		if len(object.Actions) == 0 {
+			log.Trace("%v already present on server", object.Pointer)
+			return nil
+		}
 
-			content, err := transferAdapter.Download(ctx, link)
-			if err != nil {
-				return err
-			}
+		link, ok := object.Actions["upload"]
+		if !ok {
+			return errors.New("missing action 'upload'")
+		}
+
+		content, err := uc(object.Pointer, nil)
+		if err != nil {
+			return err
+		}
 
-			if err := dc(object.Pointer, content, nil); err != nil {
+		err = transferAdapter.Upload(ctx, link, object.Pointer, content)
+		if err != nil {
+			return err
+		}
+
+		link, ok = object.Actions["verify"]
+		if ok {
+			if err := transferAdapter.Verify(ctx, link, object.Pointer); err != nil {
 				return err
 			}
 		}
-	}
+	} else {
+		link, ok := object.Actions["download"]
+		if !ok {
+			// no actions block in response, try legacy response schema
+			link, ok = object.Links["download"]
+		}
+		if !ok {
+			log.Debug("%+v", object)
+			return errors.New("missing action 'download'")
+		}
 
+		content, err := transferAdapter.Download(ctx, link)
+		if err != nil {
+			return err
+		}
+
+		if err := dc(object.Pointer, content, nil); err != nil {
+			return err
+		}
+	}
 	return nil
 }