Skip to content

Commit 30d989d

Browse files
authored
Fix container parallel upload bugs (#32022)
This PR should be replaced by #31860 in v1.23. The aim of creating this PR is to fix it in 1.22 because globallock hasn't been introduced. Fix #27640 Fix #29563 Fix #31215
1 parent b3af359 commit 30d989d

File tree

2 files changed

+31
-9
lines changed

2 files changed

+31
-9
lines changed

routers/api/packages/container/blob.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,29 @@ import (
1010
"fmt"
1111
"os"
1212
"strings"
13-
"sync"
1413

1514
"code.gitea.io/gitea/models/db"
1615
packages_model "code.gitea.io/gitea/models/packages"
1716
container_model "code.gitea.io/gitea/models/packages/container"
17+
user_model "code.gitea.io/gitea/models/user"
1818
"code.gitea.io/gitea/modules/log"
1919
packages_module "code.gitea.io/gitea/modules/packages"
2020
container_module "code.gitea.io/gitea/modules/packages/container"
21+
"code.gitea.io/gitea/modules/sync"
2122
"code.gitea.io/gitea/modules/util"
2223
packages_service "code.gitea.io/gitea/services/packages"
2324
)
2425

25-
var uploadVersionMutex sync.Mutex
26+
// TODO: use clustered lock
27+
var uploadVersionMutex = sync.NewExclusivePool()
2628

2729
// saveAsPackageBlob creates a package blob from an upload
2830
// The uploaded blob gets stored in a special upload version to link them to the package/image
2931
func saveAsPackageBlob(ctx context.Context, hsr packages_module.HashedSizeReader, pci *packages_service.PackageCreationInfo) (*packages_model.PackageBlob, error) {
32+
pkgPath := pci.PackageInfo.Owner.LowerName + "/" + pci.PackageInfo.Name
33+
uploadVersionMutex.CheckIn(pkgPath)
34+
defer uploadVersionMutex.CheckOut(pkgPath)
35+
3036
pb := packages_service.NewPackageBlob(hsr)
3137

3238
exists := false
@@ -80,6 +86,10 @@ func saveAsPackageBlob(ctx context.Context, hsr packages_module.HashedSizeReader
8086

8187
// mountBlob mounts the specific blob to a different package
8288
func mountBlob(ctx context.Context, pi *packages_service.PackageInfo, pb *packages_model.PackageBlob) error {
89+
pkgPath := pi.Owner.LowerName + "/" + pi.Name
90+
uploadVersionMutex.CheckIn(pkgPath)
91+
defer uploadVersionMutex.CheckOut(pkgPath)
92+
8393
uploadVersion, err := getOrCreateUploadVersion(ctx, pi)
8494
if err != nil {
8595
return err
@@ -93,9 +103,6 @@ func mountBlob(ctx context.Context, pi *packages_service.PackageInfo, pb *packag
93103
func getOrCreateUploadVersion(ctx context.Context, pi *packages_service.PackageInfo) (*packages_model.PackageVersion, error) {
94104
var uploadVersion *packages_model.PackageVersion
95105

96-
// FIXME: Replace usage of mutex with database transaction
97-
// https://github.com/go-gitea/gitea/pull/21862
98-
uploadVersionMutex.Lock()
99106
err := db.WithTx(ctx, func(ctx context.Context) error {
100107
created := true
101108
p := &packages_model.Package{
@@ -140,7 +147,6 @@ func getOrCreateUploadVersion(ctx context.Context, pi *packages_service.PackageI
140147

141148
return nil
142149
})
143-
uploadVersionMutex.Unlock()
144150

145151
return uploadVersion, err
146152
}
@@ -172,10 +178,14 @@ func createFileForBlob(ctx context.Context, pv *packages_model.PackageVersion, p
172178
return nil
173179
}
174180

175-
func deleteBlob(ctx context.Context, ownerID int64, image, digest string) error {
181+
func deleteBlob(ctx context.Context, owner *user_model.User, image, digest string) error {
182+
pkgPath := owner.LowerName + "/" + image
183+
uploadVersionMutex.CheckIn(pkgPath)
184+
defer uploadVersionMutex.CheckOut(pkgPath)
185+
176186
return db.WithTx(ctx, func(ctx context.Context) error {
177187
pfds, err := container_model.GetContainerBlobs(ctx, &container_model.BlobSearchOptions{
178-
OwnerID: ownerID,
188+
OwnerID: owner.ID,
179189
Image: image,
180190
Digest: digest,
181191
})

routers/api/packages/container/container.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
packages_module "code.gitea.io/gitea/modules/packages"
2525
container_module "code.gitea.io/gitea/modules/packages/container"
2626
"code.gitea.io/gitea/modules/setting"
27+
"code.gitea.io/gitea/modules/sync"
2728
"code.gitea.io/gitea/modules/util"
2829
"code.gitea.io/gitea/routers/api/packages/helper"
2930
auth_service "code.gitea.io/gitea/services/auth"
@@ -540,7 +541,7 @@ func DeleteBlob(ctx *context.Context) {
540541
return
541542
}
542543

543-
if err := deleteBlob(ctx, ctx.Package.Owner.ID, ctx.Params("image"), d); err != nil {
544+
if err := deleteBlob(ctx, ctx.Package.Owner, ctx.Params("image"), d); err != nil {
544545
apiError(ctx, http.StatusInternalServerError, err)
545546
return
546547
}
@@ -550,6 +551,9 @@ func DeleteBlob(ctx *context.Context) {
550551
})
551552
}
552553

554+
// TODO: use clustered lock
555+
var lockManifest = sync.NewExclusivePool()
556+
553557
// https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-manifests
554558
func UploadManifest(ctx *context.Context) {
555559
reference := ctx.Params("reference")
@@ -581,6 +585,10 @@ func UploadManifest(ctx *context.Context) {
581585
return
582586
}
583587

588+
imagePath := ctx.Package.Owner.Name + "/" + ctx.Params("image")
589+
lockManifest.CheckIn(imagePath)
590+
defer lockManifest.CheckOut(imagePath)
591+
584592
digest, err := processManifest(ctx, mci, buf)
585593
if err != nil {
586594
var namedError *namedError
@@ -679,6 +687,10 @@ func DeleteManifest(ctx *context.Context) {
679687
return
680688
}
681689

690+
imagePath := ctx.Package.Owner.Name + "/" + ctx.Params("image")
691+
lockManifest.CheckIn(imagePath)
692+
defer lockManifest.CheckOut(imagePath)
693+
682694
pvs, err := container_model.GetManifestVersions(ctx, opts)
683695
if err != nil {
684696
apiError(ctx, http.StatusInternalServerError, err)

0 commit comments

Comments
 (0)