mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-13 06:20:24 +00:00
[v15.0/forgejo] fix: unique key violation in first-time concurrent debian package uploads to a user (#11906)
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/11881 Fixes an intermittent test failure in `TestPackageDebianConcurrent`, [example](https://codeberg.org/forgejo/forgejo/actions/runs/148747/jobs/9/attempt/1#jobstep-5-981), introduced by testing in #11776. This one is caused by duplicate writes to `user_setting` to store a GPG key (questionable place for that...). Confirmed reproduced in local testing and test now passes: ``` === TestPackageDebianConcurrent (tests/test_utils.go:344) === TestPackageDebianConcurrent/Concurrent_Upload (tests/integration/api_packages_debian_test.go:334) ... other duplicate key violations ... // TestPackageDebianConcurrent/Concurrent_Upload "2026/03/29 10:31:57 ...dels/user/setting.go:210:func1() [E] [Error SQL Query] INSERT INTO \"gtestschema\".\"user_setting\" (\"user_id\",\"setting_key\",\"setting_value\") VALUES ($1,$2,$3) RETURNING \"id\" [2 debian.key.private -----BEGIN PGP PRIVATE KEY BLOCK----- ...snip... -----END PGP PRIVATE KEY BLOCK-----] - ERROR: duplicate key value violates unique constraint \"UQE_user_setting_key_userid\" (SQLSTATE 23505)", PASS ``` No additional test required as it is already tripping a test failure. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests for Go changes - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. (already present and failing) - I ran... - [x] `make pr-go` before pushing ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11906 Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org> Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org> Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
This commit is contained in:
parent
adebf2adac
commit
d42c66471a
1 changed files with 17 additions and 1 deletions
|
|
@ -14,6 +14,7 @@ import (
|
|||
"strings"
|
||||
"time"
|
||||
|
||||
"forgejo.org/models/db"
|
||||
packages_model "forgejo.org/models/packages"
|
||||
debian_model "forgejo.org/models/packages/debian"
|
||||
user_model "forgejo.org/models/user"
|
||||
|
|
@ -28,6 +29,7 @@ import (
|
|||
"github.com/ProtonMail/go-crypto/openpgp/clearsign"
|
||||
"github.com/ProtonMail/go-crypto/openpgp/packet"
|
||||
"github.com/ulikunitz/xz"
|
||||
"xorm.io/xorm"
|
||||
)
|
||||
|
||||
// GetOrCreateRepositoryVersion gets or creates the internal repository package
|
||||
|
|
@ -308,7 +310,21 @@ func buildReleaseFiles(ctx context.Context, ownerID int64, repoVersion *packages
|
|||
|
||||
sort.Strings(architectures)
|
||||
|
||||
priv, _, err := GetOrCreateKeyPair(ctx, ownerID)
|
||||
// ErrUniqueConstraintViolation can occur rarely when two concurrent updates occur to the same organization and
|
||||
// `GetOrCreateKeyPair` ends up being invoked simulatneously, which writes to `user_setting` to store a GPG key for
|
||||
// the `Release.gpg` file. In that event, retry the rebuild.
|
||||
//
|
||||
// See comment in package services' createPackageAndAddFile for why we cannot recover from the error in any other
|
||||
// way.
|
||||
var priv string
|
||||
err = db.RetryTx(ctx, db.RetryConfig{
|
||||
// A single retry is sufficient the user/org's key pair would have been created by the first successful tx.
|
||||
AttemptCount: 2,
|
||||
ErrorIs: []error{xorm.ErrUniqueConstraintViolation},
|
||||
}, func(ctx context.Context) error {
|
||||
priv, _, err = GetOrCreateKeyPair(ctx, ownerID)
|
||||
return err
|
||||
})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue