Skip to content

Commit d130dd6

Browse files
author
yeyuanjie
committed
Remove the db transaction and load all corresponding versions into memory, ignoring deletion errors
1 parent 277b9c5 commit d130dd6

File tree

3 files changed

+97
-111
lines changed

3 files changed

+97
-111
lines changed

‎models/packages/package_version.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -291,14 +291,14 @@ func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*Package
291291
Where(opts.ToConds())
292292

293293
opts.configureOrderBy(sess)
294-
294+
pvs := make([]*PackageVersion, 0, 10)
295295
if opts.Paginator != nil {
296296
sess = db.SetSessionPagination(sess, opts)
297+
count, err := sess.FindAndCount(&pvs)
298+
return pvs, count, err
297299
}
298-
299-
pvs := make([]*PackageVersion, 0, 10)
300-
count, err := sess.FindAndCount(&pvs)
301-
return pvs, count, err
300+
err := sess.Find(&pvs)
301+
return pvs, int64(len(pvs)), err
302302
}
303303

304304
// SearchLatestVersions gets the latest version of every package matching the search options

‎routers/web/shared/packages/packages.go‎

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"net/http"
99
"time"
1010

11-
"code.gitea.io/gitea/models/db"
1211
packages_model "code.gitea.io/gitea/models/packages"
1312
user_model "code.gitea.io/gitea/models/user"
1413
"code.gitea.io/gitea/modules/log"
@@ -155,49 +154,48 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) {
155154
versionsToRemove := make([]*packages_model.PackageDescriptor, 0, 10)
156155

157156
for _, p := range packages {
158-
skip := pcr.KeepCount
159-
for {
160-
pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{
161-
PackageID: p.ID,
162-
IsInternal: optional.Some(false),
163-
Sort: packages_model.SortCreatedDesc,
164-
Paginator: db.NewAbsoluteListOptions(skip, 200),
165-
})
166-
if err != nil {
167-
ctx.ServerError("SearchVersions", err)
157+
pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{
158+
PackageID: p.ID,
159+
IsInternal: optional.Some(false),
160+
Sort: packages_model.SortCreatedDesc,
161+
})
162+
if err != nil {
163+
ctx.ServerError("SearchVersions", err)
164+
return
165+
}
166+
if pcr.KeepCount > 0 {
167+
if pcr.KeepCount < len(pvs) {
168+
pvs = pvs[pcr.KeepCount:]
169+
} else {
170+
pvs = nil
171+
}
172+
}
173+
for _, pv := range pvs {
174+
if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil {
175+
ctx.ServerError("ShouldBeSkipped", err)
168176
return
177+
} else if skip {
178+
continue
169179
}
170-
if len(pvs) == 0 {
171-
break
180+
toMatch := pv.LowerVersion
181+
if pcr.MatchFullName {
182+
toMatch = p.LowerName + "/" + pv.LowerVersion
172183
}
173-
for _, pv := range pvs {
174-
skip++
175-
if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil {
176-
ctx.ServerError("ShouldBeSkipped", err)
177-
return
178-
} else if skip {
179-
continue
180-
}
181-
toMatch := pv.LowerVersion
182-
if pcr.MatchFullName {
183-
toMatch = p.LowerName + "/" + pv.LowerVersion
184-
}
185-
if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) {
186-
continue
187-
}
188-
if pv.CreatedUnix.AsLocalTime().After(olderThan) {
189-
continue
190-
}
191-
if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) {
192-
continue
193-
}
194-
pd, err := packages_model.GetPackageDescriptor(ctx, pv)
195-
if err != nil {
196-
ctx.ServerError("GetPackageDescriptor", err)
197-
return
198-
}
199-
versionsToRemove = append(versionsToRemove, pd)
184+
if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) {
185+
continue
186+
}
187+
if pv.CreatedUnix.AsLocalTime().After(olderThan) {
188+
continue
189+
}
190+
if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) {
191+
continue
192+
}
193+
pd, err := packages_model.GetPackageDescriptor(ctx, pv)
194+
if err != nil {
195+
ctx.ServerError("GetPackageDescriptor", err)
196+
return
200197
}
198+
versionsToRemove = append(versionsToRemove, pd)
201199
}
202200
}
203201

‎services/packages/cleanup/cleanup.go‎

Lines changed: 54 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,7 @@ func CleanupTask(ctx context.Context, olderThan time.Duration) error {
3333
}
3434

3535
func ExecuteCleanupRules(outerCtx context.Context) error {
36-
ctx, committer, err := db.TxContext(outerCtx)
37-
if err != nil {
38-
return err
39-
}
40-
defer committer.Close()
41-
42-
err = packages_model.IterateEnabledCleanupRules(ctx, func(ctx context.Context, pcr *packages_model.PackageCleanupRule) error {
36+
return packages_model.IterateEnabledCleanupRules(outerCtx, func(ctx context.Context, pcr *packages_model.PackageCleanupRule) error {
4337
select {
4438
case <-outerCtx.Done():
4539
return db.ErrCancelledf("While processing package cleanup rules")
@@ -59,64 +53,63 @@ func ExecuteCleanupRules(outerCtx context.Context) error {
5953

6054
anyVersionDeleted := false
6155
for _, p := range packages {
62-
skip := pcr.KeepCount
63-
for {
64-
pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{
65-
PackageID: p.ID,
66-
IsInternal: optional.Some(false),
67-
Sort: packages_model.SortCreatedDesc,
68-
Paginator: db.NewAbsoluteListOptions(skip, 200),
69-
})
70-
if err != nil {
71-
return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err)
72-
}
73-
if len(pvs) == 0 {
74-
break
56+
pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{
57+
PackageID: p.ID,
58+
IsInternal: optional.Some(false),
59+
Sort: packages_model.SortCreatedDesc,
60+
})
61+
if err != nil {
62+
return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err)
63+
}
64+
if pcr.KeepCount > 0 {
65+
if pcr.KeepCount < len(pvs) {
66+
pvs = pvs[pcr.KeepCount:]
67+
} else {
68+
pvs = nil
7569
}
76-
versionDeleted := false
77-
skip += len(pvs)
78-
for _, pv := range pvs {
79-
if pcr.Type == packages_model.TypeContainer {
80-
if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil {
81-
return fmt.Errorf("CleanupRule [%d]: container.ShouldBeSkipped failed: %w", pcr.ID, err)
82-
} else if skip {
83-
log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version)
84-
continue
85-
}
86-
}
87-
toMatch := pv.LowerVersion
88-
if pcr.MatchFullName {
89-
toMatch = p.LowerName + "/" + pv.LowerVersion
90-
}
91-
if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) {
92-
log.Debug("Rule[%d]: keep '%s/%s' (keep pattern)", pcr.ID, p.Name, pv.Version)
93-
continue
94-
}
95-
if pv.CreatedUnix.AsLocalTime().After(olderThan) {
96-
log.Debug("Rule[%d]: keep '%s/%s' (remove days) %v", pcr.ID, p.Name, pv.Version, pv.CreatedUnix.FormatDate())
97-
continue
98-
}
99-
if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) {
100-
log.Debug("Rule[%d]: keep '%s/%s' (remove pattern)", pcr.ID, p.Name, pv.Version)
70+
}
71+
versionDeleted := false
72+
for _, pv := range pvs {
73+
if pcr.Type == packages_model.TypeContainer {
74+
if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil {
75+
return fmt.Errorf("CleanupRule [%d]: container.ShouldBeSkipped failed: %w", pcr.ID, err)
76+
} else if skip {
77+
log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version)
10178
continue
10279
}
103-
log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version)
104-
if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil {
105-
return fmt.Errorf("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err)
106-
}
107-
skip--
108-
versionDeleted = true
109-
anyVersionDeleted = true
11080
}
111-
if versionDeleted {
112-
if pcr.Type == packages_model.TypeCargo {
113-
owner, err := user_model.GetUserByID(ctx, pcr.OwnerID)
114-
if err != nil {
115-
return fmt.Errorf("GetUserByID failed: %w", err)
116-
}
117-
if err := cargo_service.UpdatePackageIndexIfExists(ctx, owner, owner, p.ID); err != nil {
118-
return fmt.Errorf("CleanupRule [%d]: cargo.UpdatePackageIndexIfExists failed: %w", pcr.ID, err)
119-
}
81+
toMatch := pv.LowerVersion
82+
if pcr.MatchFullName {
83+
toMatch = p.LowerName + "/" + pv.LowerVersion
84+
}
85+
if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) {
86+
log.Debug("Rule[%d]: keep '%s/%s' (keep pattern)", pcr.ID, p.Name, pv.Version)
87+
continue
88+
}
89+
if pv.CreatedUnix.AsLocalTime().After(olderThan) {
90+
log.Debug("Rule[%d]: keep '%s/%s' (remove days) %v", pcr.ID, p.Name, pv.Version, pv.CreatedUnix.FormatDate())
91+
continue
92+
}
93+
if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) {
94+
log.Debug("Rule[%d]: keep '%s/%s' (remove pattern)", pcr.ID, p.Name, pv.Version)
95+
continue
96+
}
97+
log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version)
98+
if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil {
99+
log.Error("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err)
100+
continue
101+
}
102+
versionDeleted = true
103+
anyVersionDeleted = true
104+
}
105+
if versionDeleted {
106+
if pcr.Type == packages_model.TypeCargo {
107+
owner, err := user_model.GetUserByID(ctx, pcr.OwnerID)
108+
if err != nil {
109+
return fmt.Errorf("GetUserByID failed: %w", err)
110+
}
111+
if err := cargo_service.UpdatePackageIndexIfExists(ctx, owner, owner, p.ID); err != nil {
112+
return fmt.Errorf("CleanupRule [%d]: cargo.UpdatePackageIndexIfExists failed: %w", pcr.ID, err)
120113
}
121114
}
122115
}
@@ -150,11 +143,6 @@ func ExecuteCleanupRules(outerCtx context.Context) error {
150143
}
151144
return nil
152145
})
153-
if err != nil {
154-
return err
155-
}
156-
157-
return committer.Commit()
158146
}
159147

160148
func CleanupExpiredData(outerCtx context.Context, olderThan time.Duration) error {

0 commit comments

Comments
 (0)