Merge pull request #12538 from heww/db-tx-skippers

perf(db): skip tx for get blob, patch/put blob upload apis

src/core/middlewares/middlewares.go

@@ -15,8 +15,11 @@
 package middlewares
 
 import (
+	"net/http"
+	"regexp"
+
 	"github.com/astaxie/beego"
-	"github.com/docker/distribution/reference"
+	"github.com/goharbor/harbor/src/pkg/distribution"
 	"github.com/goharbor/harbor/src/server/middleware"
 	"github.com/goharbor/harbor/src/server/middleware/csrf"
 	"github.com/goharbor/harbor/src/server/middleware/log"
@@ -27,19 +30,22 @@ import (
 	"github.com/goharbor/harbor/src/server/middleware/security"
 	"github.com/goharbor/harbor/src/server/middleware/session"
 	"github.com/goharbor/harbor/src/server/middleware/transaction"
-	"net/http"
-	"regexp"
 )
 
 var (
 	match         = regexp.MustCompile
 	numericRegexp = match(`[0-9]+`)
 
-	blobURLRe = match("^/v2/(" + reference.NameRegexp.String() + ")/blobs/" + reference.DigestRegexp.String())
-
-	// fetchBlobAPISkipper skip transaction middleware for fetch blob API
-	// because transaction use the ResponseBuffer for the response which will degrade the performance for fetch blob
-	fetchBlobAPISkipper = middleware.MethodAndPathSkipper(http.MethodGet, blobURLRe)
+	// dbTxSkippers skip the transaction middleware for GET Blob, PATCH Blob Upload and PUT Blob Upload APIs
+	// because the APIs may take a long time to run, enable the transaction middleware in them will hold the database connections
+	// until the API finished, this behavior may eat all the database connections.
+	// There are no database writing operations in the GET Blob and PATCH Blob APIs, so skip the transaction middleware is all ok.
+	// For the PUT Blob Upload API, we will make a transaction manually to write blob info to the database when put blob upload successfully.
+	dbTxSkippers = []middleware.Skipper{
+		middleware.MethodAndPathSkipper(http.MethodGet, distribution.BlobURLRegexp),
+		middleware.MethodAndPathSkipper(http.MethodPatch, distribution.BlobUploadURLRegexp),
+		middleware.MethodAndPathSkipper(http.MethodPut, distribution.BlobUploadURLRegexp),
+	}
 
 	// readonlySkippers skip the post request when harbor sets to readonly.
 	readonlySkippers = []middleware.Skipper{
@@ -65,11 +71,10 @@ func MiddleWares() []beego.MiddleWare {
 		log.Middleware(),
 		session.Middleware(),
 		csrf.Middleware(),
+		orm.Middleware(),
+		notification.Middleware(), // notification must ahead of transaction ensure the DB transaction execution complete
+		transaction.Middleware(dbTxSkippers...),
 		security.Middleware(),
 		readonly.Middleware(readonlySkippers...),
-		orm.Middleware(),
-		// notification must ahead of transaction ensure the DB transaction execution complete
-		notification.Middleware(),
-		transaction.Middleware(fetchBlobAPISkipper),
 	}
 }

src/core/middlewares/middlewares_test.go

@@ -20,28 +20,6 @@ import (
 	"testing"
 )
 
-func Test_fetchBlobAPISkipper(t *testing.T) {
-	type args struct {
-		r *http.Request
-	}
-	tests := []struct {
-		name string
-		args args
-		want bool
-	}{
-		{"fetch blob", args{httptest.NewRequest(http.MethodGet, "/v2/library/photon/blobs/sha256:6e0447537050cf871f9ab6a3fec5715f9c6fff5212f6666993f1fc46b1f717a3", nil)}, true},
-		{"delete blob", args{httptest.NewRequest(http.MethodDelete, "/v2/library/photon/blobs/sha256:6e0447537050cf871f9ab6a3fec5715f9c6fff5212f6666993f1fc46b1f717a3", nil)}, false},
-		{"get manifest", args{httptest.NewRequest(http.MethodDelete, "/v2/library/photon/manifests/sha256:6e0447537050cf871f9ab6a3fec5715f9c6fff5212f6666993f1fc46b1f717a3", nil)}, false},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := fetchBlobAPISkipper(tt.args.r); got != tt.want {
-				t.Errorf("fetchBlobAPISkipper() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 func Test_readonlySkipper(t *testing.T) {
 	type args struct {
 		r *http.Request

src/pkg/distribution/distribution.go

@@ -47,8 +47,12 @@ var (
 var (
 	name      = fmt.Sprintf("(?P<name>%s)", ref.NameRegexp)
 	reference = fmt.Sprintf("(?P<reference>((%s)|(%s)))", ref.DigestRegexp, ref.TagRegexp)
+	dgt       = fmt.Sprintf("(?P<digest>%s)", ref.DigestRegexp)
 	sessionID = "(?P<session_id>[a-zA-Z0-9-_.=]+)"
 
+	// BlobURLRegexp regexp which match blob url
+	BlobURLRegexp = regexp.MustCompile(`^/v2/` + name + `/blobs/` + dgt)
+
 	// BlobUploadURLRegexp regexp which match blob upload url
 	BlobUploadURLRegexp = regexp.MustCompile(`^/v2/` + name + `/blobs/uploads/` + sessionID)
 

src/server/middleware/blob/put_blob_upload.go

@@ -15,11 +15,14 @@
 package blob
 
 import (
-	"github.com/goharbor/harbor/src/lib/log"
-	"github.com/goharbor/harbor/src/pkg/distribution"
-	"github.com/goharbor/harbor/src/server/middleware"
+	"context"
 	"net/http"
 	"strconv"
+
+	"github.com/goharbor/harbor/src/lib/log"
+	"github.com/goharbor/harbor/src/lib/orm"
+	"github.com/goharbor/harbor/src/pkg/distribution"
+	"github.com/goharbor/harbor/src/server/middleware"
 )
 
 // PutBlobUploadMiddleware middleware is to update the blob status according to the different situation before the request passed into proxy(distribution).
@@ -42,36 +45,40 @@ func PutBlobUploadMiddleware() func(http.Handler) http.Handler {
 
 		ctx := r.Context()
 
-		logger := log.G(ctx).WithFields(log.Fields{"middleware": "blob"})
+		h := func(ctx context.Context) error {
+			logger := log.G(ctx).WithFields(log.Fields{"middleware": "blob"})
 
-		size, err := strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64)
-		if err != nil || size == 0 {
-			size, err = blobController.GetAcceptedBlobSize(distribution.ParseSessionID(r.URL.Path))
-		}
-		if err != nil {
-			logger.Errorf("get blob size failed, error: %v", err)
-			return err
+			size, err := strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64)
+			if err != nil || size == 0 {
+				size, err = blobController.GetAcceptedBlobSize(distribution.ParseSessionID(r.URL.Path))
+			}
+			if err != nil {
+				logger.Errorf("get blob size failed, error: %v", err)
+				return err
+			}
+
+			p, err := projectController.GetByName(ctx, distribution.ParseProjectName(r.URL.Path))
+			if err != nil {
+				logger.Errorf("get project failed, error: %v", err)
+				return err
+			}
+
+			digest := w.Header().Get("Docker-Content-Digest")
+			blobID, err := blobController.Ensure(ctx, digest, "application/octet-stream", size)
+			if err != nil {
+				logger.Errorf("ensure blob %s failed, error: %v", digest, err)
+				return err
+			}
+
+			if err := blobController.AssociateWithProjectByID(ctx, blobID, p.ProjectID); err != nil {
+				logger.Errorf("associate blob %s with project %s failed, error: %v", digest, p.Name, err)
+				return err
+			}
+
+			return nil
 		}
 
-		p, err := projectController.GetByName(ctx, distribution.ParseProjectName(r.URL.Path))
-		if err != nil {
-			logger.Errorf("get project failed, error: %v", err)
-			return err
-		}
-
-		digest := w.Header().Get("Docker-Content-Digest")
-		blobID, err := blobController.Ensure(ctx, digest, "application/octet-stream", size)
-		if err != nil {
-			logger.Errorf("ensure blob %s failed, error: %v", digest, err)
-			return err
-		}
-
-		if err := blobController.AssociateWithProjectByID(ctx, blobID, p.ProjectID); err != nil {
-			logger.Errorf("associate blob %s with project %s failed, error: %v", digest, p.Name, err)
-			return err
-		}
-
-		return nil
+		return orm.WithTransaction(h)(ctx)
 	})
 
 	return middleware.Chain(before, after)