loading up the forgejo repo on tangled to test page performance
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Fail mirroring more gracefully (#34002)

* reuse recoverable error checks across mirror_pull
* add new cases for 'cannot lock ref/not our ref' (race condition in
fetch) and 'Unable to create/lock"
* move lfs sync right after commit graph write, and before other
maintenance which may fail
* try a prune for 'broken reference' as well as 'not our ref'
* always sync LFS right after commit graph write, and before other
maintenance which may fail

This handles a few cases where our very large and very active
repositories could serve mirrored git refs, but be missing lfs files:

## Case 1 (multiple variants): Race condition in git fetch
There was already a check for 'unable to resolve reference' on a failed
git fetch, after which a git prune and then subsequent fetch are
performed. This is to work around a race condition where the git remote
tells Gitea about a ref for some HEAD of a branch, then fails a few
seconds later because the remote branch was deleted, or the ref was
updated (force push).

There are two more variants to the error message you can get, but for
the same kind of race condition. These *may* be related to the git
binary version Gitea has access to (in my case, it was 2.48.1).

## Case 2: githttp.go can serve updated git refs before it's synced lfs
oids

There is probably a more aggressive refactor we could do here to have
the cat-file loop use FETCH_HEAD instead of relying on the commit graphs
to be committed locally (and thus serveable to clients of Gitea), but a
simple reduction in the occurrences of this for me was to move the lfs
sync block immediately after the commit-graph write and before any other
time-consuming (or potentially erroring/exiting) blocks.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
(cherry picked from commit e0ad72e2233f885669c26d9063a91abd594fb9f6)

authored by

Royce Remer
wxiaoguang
and committed by
Gusted
adc2a215 6d5fc194

+123 -77
+29 -11
services/mirror/mirror_pull.go
··· 244 244 return pruneErr 245 245 } 246 246 247 + // checkRecoverableSyncError takes an error message from a git fetch command and returns false if it should be a fatal/blocking error 248 + func checkRecoverableSyncError(stderrMessage string) bool { 249 + switch { 250 + case strings.Contains(stderrMessage, "unable to resolve reference") && strings.Contains(stderrMessage, "reference broken"): 251 + return true 252 + case strings.Contains(stderrMessage, "remote error") && strings.Contains(stderrMessage, "not our ref"): 253 + return true 254 + case strings.Contains(stderrMessage, "cannot lock ref") && strings.Contains(stderrMessage, "but expected"): 255 + return true 256 + case strings.Contains(stderrMessage, "cannot lock ref") && strings.Contains(stderrMessage, "unable to resolve reference"): 257 + return true 258 + case strings.Contains(stderrMessage, "Unable to create") && strings.Contains(stderrMessage, ".lock"): 259 + return true 260 + default: 261 + return false 262 + } 263 + } 264 + 247 265 // runSync returns true if sync finished without error. 248 266 func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bool) { 249 267 repoPath := m.Repo.RepoPath() ··· 286 304 stdoutMessage := util.SanitizeCredentialURLs(stdout) 287 305 288 306 // Now check if the error is a resolve reference due to broken reference 289 - if strings.Contains(stderr, "unable to resolve reference") && strings.Contains(stderr, "reference broken") { 307 + if checkRecoverableSyncError(stderr) { 290 308 log.Warn("SyncMirrors [repo: %-v]: failed to update mirror repository due to broken references:\nStdout: %s\nStderr: %s\nErr: %v\nAttempting Prune", m.Repo, stdoutMessage, stderrMessage, err) 291 309 err = nil 292 310 ··· 337 355 return nil, false 338 356 } 339 357 358 + if m.LFS && setting.LFS.StartServer { 359 + log.Trace("SyncMirrors [repo: %-v]: syncing LFS objects...", m.Repo) 360 + endpoint := lfs.DetermineEndpoint(remoteURL.String(), m.LFSEndpoint) 361 + lfsClient := lfs.NewClient(endpoint, nil) 362 + if err = repo_module.StoreMissingLfsObjectsInRepository(ctx, m.Repo, gitRepo, lfsClient); err != nil { 363 + log.Error("SyncMirrors [repo: %-v]: failed to synchronize LFS objects for repository: %v", m.Repo, err) 364 + } 365 + } 366 + 340 367 log.Trace("SyncMirrors [repo: %-v]: syncing branches...", m.Repo) 341 368 if _, err = repo_module.SyncRepoBranchesWithRepo(ctx, m.Repo, gitRepo, 0); err != nil { 342 369 log.Error("SyncMirrors [repo: %-v]: failed to synchronize branches: %v", m.Repo, err) ··· 345 372 log.Trace("SyncMirrors [repo: %-v]: syncing releases with tags...", m.Repo) 346 373 if err = repo_module.SyncReleasesWithTags(ctx, m.Repo, gitRepo); err != nil { 347 374 log.Error("SyncMirrors [repo: %-v]: failed to synchronize tags to releases: %v", m.Repo, err) 348 - } 349 - 350 - if m.LFS && setting.LFS.StartServer { 351 - log.Trace("SyncMirrors [repo: %-v]: syncing LFS objects...", m.Repo) 352 - endpoint := lfs.DetermineEndpoint(remoteURL.String(), m.LFSEndpoint) 353 - lfsClient := lfs.NewClient(endpoint, nil) 354 - if err = repo_module.StoreMissingLfsObjectsInRepository(ctx, m.Repo, gitRepo, lfsClient); err != nil { 355 - log.Error("SyncMirrors [repo: %-v]: failed to synchronize LFS objects for repository: %v", m.Repo, err) 356 - } 357 375 } 358 376 gitRepo.Close() 359 377 ··· 382 400 stdoutMessage := util.SanitizeCredentialURLs(stdout) 383 401 384 402 // Now check if the error is a resolve reference due to broken reference 385 - if strings.Contains(stderrMessage, "unable to resolve reference") && strings.Contains(stderrMessage, "reference broken") { 403 + if checkRecoverableSyncError(stderrMessage) { 386 404 log.Warn("SyncMirrors [repo: %-v Wiki]: failed to update mirror wiki repository due to broken references:\nStdout: %s\nStderr: %s\nErr: %v\nAttempting Prune", m.Repo, stdoutMessage, stderrMessage, err) 387 405 err = nil 388 406
+94
services/mirror/mirror_pull_test.go
··· 1 + // Copyright 2023 The Gitea Authors. All rights reserved. 2 + // SPDX-License-Identifier: MIT 3 + 4 + package mirror 5 + 6 + import ( 7 + "testing" 8 + 9 + "github.com/stretchr/testify/assert" 10 + ) 11 + 12 + func Test_parseRemoteUpdateOutput(t *testing.T) { 13 + output := ` 14 + * [new tag] v0.1.8 -> v0.1.8 15 + * [new branch] master -> origin/master 16 + - [deleted] (none) -> origin/test1 17 + - [deleted] (none) -> tag1 18 + + f895a1e...957a993 test2 -> origin/test2 (forced update) 19 + 957a993..a87ba5f test3 -> origin/test3 20 + * [new ref] refs/pull/26595/head -> refs/pull/26595/head 21 + * [new ref] refs/pull/26595/merge -> refs/pull/26595/merge 22 + e0639e38fb..6db2410489 refs/pull/25873/head -> refs/pull/25873/head 23 + + 1c97ebc746...976d27d52f refs/pull/25873/merge -> refs/pull/25873/merge (forced update) 24 + ` 25 + results := parseRemoteUpdateOutput(output, "origin") 26 + assert.Len(t, results, 10) 27 + assert.Equal(t, "refs/tags/v0.1.8", results[0].refName.String()) 28 + assert.Equal(t, gitShortEmptySha, results[0].oldCommitID) 29 + assert.Empty(t, results[0].newCommitID) 30 + 31 + assert.Equal(t, "refs/heads/master", results[1].refName.String()) 32 + assert.Equal(t, gitShortEmptySha, results[1].oldCommitID) 33 + assert.Empty(t, results[1].newCommitID) 34 + 35 + assert.Equal(t, "refs/heads/test1", results[2].refName.String()) 36 + assert.Empty(t, results[2].oldCommitID) 37 + assert.Equal(t, gitShortEmptySha, results[2].newCommitID) 38 + 39 + assert.Equal(t, "refs/tags/tag1", results[3].refName.String()) 40 + assert.Empty(t, results[3].oldCommitID) 41 + assert.Equal(t, gitShortEmptySha, results[3].newCommitID) 42 + 43 + assert.Equal(t, "refs/heads/test2", results[4].refName.String()) 44 + assert.Equal(t, "f895a1e", results[4].oldCommitID) 45 + assert.Equal(t, "957a993", results[4].newCommitID) 46 + 47 + assert.Equal(t, "refs/heads/test3", results[5].refName.String()) 48 + assert.Equal(t, "957a993", results[5].oldCommitID) 49 + assert.Equal(t, "a87ba5f", results[5].newCommitID) 50 + 51 + assert.Equal(t, "refs/pull/26595/head", results[6].refName.String()) 52 + assert.Equal(t, gitShortEmptySha, results[6].oldCommitID) 53 + assert.Empty(t, results[6].newCommitID) 54 + 55 + assert.Equal(t, "refs/pull/26595/merge", results[7].refName.String()) 56 + assert.Equal(t, gitShortEmptySha, results[7].oldCommitID) 57 + assert.Empty(t, results[7].newCommitID) 58 + 59 + assert.Equal(t, "refs/pull/25873/head", results[8].refName.String()) 60 + assert.Equal(t, "e0639e38fb", results[8].oldCommitID) 61 + assert.Equal(t, "6db2410489", results[8].newCommitID) 62 + 63 + assert.Equal(t, "refs/pull/25873/merge", results[9].refName.String()) 64 + assert.Equal(t, "1c97ebc746", results[9].oldCommitID) 65 + assert.Equal(t, "976d27d52f", results[9].newCommitID) 66 + } 67 + 68 + func Test_checkRecoverableSyncError(t *testing.T) { 69 + cases := []struct { 70 + recoverable bool 71 + message string 72 + }{ 73 + // A race condition in http git-fetch where certain refs were listed on the remote and are no longer there, would exit status 128 74 + {true, "fatal: remote error: upload-pack: not our ref 988881adc9fc3655077dc2d4d757d480b5ea0e11"}, 75 + // A race condition where a local gc/prune removes a named ref during a git-fetch would exit status 1 76 + {true, "cannot lock ref 'refs/pull/123456/merge': unable to resolve reference 'refs/pull/134153/merge'"}, 77 + // A race condition in http git-fetch where named refs were listed on the remote and are no longer there 78 + {true, "error: cannot lock ref 'refs/remotes/origin/foo': unable to resolve reference 'refs/remotes/origin/foo': reference broken"}, 79 + // A race condition in http git-fetch where named refs were force-pushed during the update, would exit status 128 80 + {true, "error: cannot lock ref 'refs/pull/123456/merge': is at 988881adc9fc3655077dc2d4d757d480b5ea0e11 but expected 7f894307ffc9553edbd0b671cab829786866f7b2"}, 81 + // A race condition with other local git operations, such as git-maintenance, would exit status 128 (well, "Unable" the "U" is uppercase) 82 + {true, "fatal: Unable to create '/data/gitea-repositories/foo-org/bar-repo.git/./objects/info/commit-graphs/commit-graph-chain.lock': File exists."}, 83 + // Missing or unauthorized credentials, would exit status 128 84 + {false, "fatal: Authentication failed for 'https://example.com/foo-does-not-exist/bar.git/'"}, 85 + // A non-existent remote repository, would exit status 128 86 + {false, "fatal: Could not read from remote repository."}, 87 + // A non-functioning proxy, would exit status 128 88 + {false, "fatal: unable to access 'https://example.com/foo-does-not-exist/bar.git/': Failed to connect to configured-https-proxy port 1080 after 0 ms: Couldn't connect to server"}, 89 + } 90 + 91 + for _, c := range cases { 92 + assert.Equal(t, c.recoverable, checkRecoverableSyncError(c.message), "test case: %s", c.message) 93 + } 94 + }
-66
services/mirror/mirror_test.go
··· 1 - // Copyright 2023 The Gitea Authors. All rights reserved. 2 - // SPDX-License-Identifier: MIT 3 - 4 - package mirror 5 - 6 - import ( 7 - "testing" 8 - 9 - "github.com/stretchr/testify/assert" 10 - ) 11 - 12 - func Test_parseRemoteUpdateOutput(t *testing.T) { 13 - output := ` 14 - * [new tag] v0.1.8 -> v0.1.8 15 - * [new branch] master -> origin/master 16 - - [deleted] (none) -> origin/test1 17 - - [deleted] (none) -> tag1 18 - + f895a1e...957a993 test2 -> origin/test2 (forced update) 19 - 957a993..a87ba5f test3 -> origin/test3 20 - * [new ref] refs/pull/26595/head -> refs/pull/26595/head 21 - * [new ref] refs/pull/26595/merge -> refs/pull/26595/merge 22 - e0639e38fb..6db2410489 refs/pull/25873/head -> refs/pull/25873/head 23 - + 1c97ebc746...976d27d52f refs/pull/25873/merge -> refs/pull/25873/merge (forced update) 24 - ` 25 - results := parseRemoteUpdateOutput(output, "origin") 26 - assert.Len(t, results, 10) 27 - assert.Equal(t, "refs/tags/v0.1.8", results[0].refName.String()) 28 - assert.Equal(t, gitShortEmptySha, results[0].oldCommitID) 29 - assert.Empty(t, results[0].newCommitID) 30 - 31 - assert.Equal(t, "refs/heads/master", results[1].refName.String()) 32 - assert.Equal(t, gitShortEmptySha, results[1].oldCommitID) 33 - assert.Empty(t, results[1].newCommitID) 34 - 35 - assert.Equal(t, "refs/heads/test1", results[2].refName.String()) 36 - assert.Empty(t, results[2].oldCommitID) 37 - assert.Equal(t, gitShortEmptySha, results[2].newCommitID) 38 - 39 - assert.Equal(t, "refs/tags/tag1", results[3].refName.String()) 40 - assert.Empty(t, results[3].oldCommitID) 41 - assert.Equal(t, gitShortEmptySha, results[3].newCommitID) 42 - 43 - assert.Equal(t, "refs/heads/test2", results[4].refName.String()) 44 - assert.Equal(t, "f895a1e", results[4].oldCommitID) 45 - assert.Equal(t, "957a993", results[4].newCommitID) 46 - 47 - assert.Equal(t, "refs/heads/test3", results[5].refName.String()) 48 - assert.Equal(t, "957a993", results[5].oldCommitID) 49 - assert.Equal(t, "a87ba5f", results[5].newCommitID) 50 - 51 - assert.Equal(t, "refs/pull/26595/head", results[6].refName.String()) 52 - assert.Equal(t, gitShortEmptySha, results[6].oldCommitID) 53 - assert.Empty(t, results[6].newCommitID) 54 - 55 - assert.Equal(t, "refs/pull/26595/merge", results[7].refName.String()) 56 - assert.Equal(t, gitShortEmptySha, results[7].oldCommitID) 57 - assert.Empty(t, results[7].newCommitID) 58 - 59 - assert.Equal(t, "refs/pull/25873/head", results[8].refName.String()) 60 - assert.Equal(t, "e0639e38fb", results[8].oldCommitID) 61 - assert.Equal(t, "6db2410489", results[8].newCommitID) 62 - 63 - assert.Equal(t, "refs/pull/25873/merge", results[9].refName.String()) 64 - assert.Equal(t, "1c97ebc746", results[9].oldCommitID) 65 - assert.Equal(t, "976d27d52f", results[9].newCommitID) 66 - }