Skip to content

Commit d03dd04

Browse files
GiteaBotlunny
andauthored
Remove transaction for archive download (#32186) (#32520)
Backport #32186 by @lunny Since there is a status column in the database, the transaction is unnecessary when downloading an archive. The transaction is blocking database operations, especially with SQLite. Replace #27563 Co-authored-by: Lunny Xiao <[email protected]>
1 parent 257ce61 commit d03dd04

File tree

2 files changed

+19
-26
lines changed

2 files changed

+19
-26
lines changed

services/repository/archiver/archiver.go

+13-20
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func (e RepoRefNotFoundError) Is(err error) bool {
6868
}
6969

7070
// NewRequest creates an archival request, based on the URI. The
71-
// resulting ArchiveRequest is suitable for being passed to ArchiveRepository()
71+
// resulting ArchiveRequest is suitable for being passed to Await()
7272
// if it's determined that the request still needs to be satisfied.
7373
func NewRequest(repoID int64, repo *git.Repository, uri string) (*ArchiveRequest, error) {
7474
r := &ArchiveRequest{
@@ -151,13 +151,14 @@ func (aReq *ArchiveRequest) Await(ctx context.Context) (*repo_model.RepoArchiver
151151
}
152152
}
153153

154+
// doArchive satisfies the ArchiveRequest being passed in. Processing
155+
// will occur in a separate goroutine, as this phase may take a while to
156+
// complete. If the archive already exists, doArchive will not do
157+
// anything. In all cases, the caller should be examining the *ArchiveRequest
158+
// being returned for completion, as it may be different than the one they passed
159+
// in.
154160
func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver, error) {
155-
txCtx, committer, err := db.TxContext(ctx)
156-
if err != nil {
157-
return nil, err
158-
}
159-
defer committer.Close()
160-
ctx, _, finished := process.GetManager().AddContext(txCtx, fmt.Sprintf("ArchiveRequest[%d]: %s", r.RepoID, r.GetArchiveName()))
161+
ctx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("ArchiveRequest[%d]: %s", r.RepoID, r.GetArchiveName()))
161162
defer finished()
162163

163164
archiver, err := repo_model.GetRepoArchiver(ctx, r.RepoID, r.Type, r.CommitID)
@@ -192,7 +193,7 @@ func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver
192193
return nil, err
193194
}
194195
}
195-
return archiver, committer.Commit()
196+
return archiver, nil
196197
}
197198

198199
if !errors.Is(err, os.ErrNotExist) {
@@ -261,17 +262,7 @@ func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver
261262
}
262263
}
263264

264-
return archiver, committer.Commit()
265-
}
266-
267-
// ArchiveRepository satisfies the ArchiveRequest being passed in. Processing
268-
// will occur in a separate goroutine, as this phase may take a while to
269-
// complete. If the archive already exists, ArchiveRepository will not do
270-
// anything. In all cases, the caller should be examining the *ArchiveRequest
271-
// being returned for completion, as it may be different than the one they passed
272-
// in.
273-
func ArchiveRepository(ctx context.Context, request *ArchiveRequest) (*repo_model.RepoArchiver, error) {
274-
return doArchive(ctx, request)
265+
return archiver, nil
275266
}
276267

277268
var archiverQueue *queue.WorkerPoolQueue[*ArchiveRequest]
@@ -281,8 +272,10 @@ func Init(ctx context.Context) error {
281272
handler := func(items ...*ArchiveRequest) []*ArchiveRequest {
282273
for _, archiveReq := range items {
283274
log.Trace("ArchiverData Process: %#v", archiveReq)
284-
if _, err := doArchive(ctx, archiveReq); err != nil {
275+
if archiver, err := doArchive(ctx, archiveReq); err != nil {
285276
log.Error("Archive %v failed: %v", archiveReq, err)
277+
} else {
278+
log.Trace("ArchiverData Success: %#v", archiver)
286279
}
287280
}
288281
return nil

services/repository/archiver/archiver_test.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,13 @@ func TestArchive_Basic(t *testing.T) {
8080
inFlight[1] = tgzReq
8181
inFlight[2] = secondReq
8282

83-
ArchiveRepository(db.DefaultContext, zipReq)
84-
ArchiveRepository(db.DefaultContext, tgzReq)
85-
ArchiveRepository(db.DefaultContext, secondReq)
83+
doArchive(db.DefaultContext, zipReq)
84+
doArchive(db.DefaultContext, tgzReq)
85+
doArchive(db.DefaultContext, secondReq)
8686

8787
// Make sure sending an unprocessed request through doesn't affect the queue
8888
// count.
89-
ArchiveRepository(db.DefaultContext, zipReq)
89+
doArchive(db.DefaultContext, zipReq)
9090

9191
// Sleep two seconds to make sure the queue doesn't change.
9292
time.Sleep(2 * time.Second)
@@ -101,15 +101,15 @@ func TestArchive_Basic(t *testing.T) {
101101
// We still have the other three stalled at completion, waiting to remove
102102
// from archiveInProgress. Try to submit this new one before its
103103
// predecessor has cleared out of the queue.
104-
ArchiveRepository(db.DefaultContext, zipReq2)
104+
doArchive(db.DefaultContext, zipReq2)
105105

106106
// Now we'll submit a request and TimedWaitForCompletion twice, before and
107107
// after we release it. We should trigger both the timeout and non-timeout
108108
// cases.
109109
timedReq, err := NewRequest(ctx.Repo.Repository.ID, ctx.Repo.GitRepo, secondCommit+".tar.gz")
110110
assert.NoError(t, err)
111111
assert.NotNil(t, timedReq)
112-
ArchiveRepository(db.DefaultContext, timedReq)
112+
doArchive(db.DefaultContext, timedReq)
113113

114114
zipReq2, err = NewRequest(ctx.Repo.Repository.ID, ctx.Repo.GitRepo, firstCommit+".zip")
115115
assert.NoError(t, err)

0 commit comments

Comments
 (0)