refactor(kvstore): share one guardRepo between merge requests and CI
- commit
- f1b7a40b1e57aab94f4f9492190da76e86f2e259
- author
- brice · · 2026-10-12 19:23 UTC
- parent
- 6d7ddba
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +29 -29
M internal/adapters/kvstore/ci.go +3 -9
| @@ -460,16 +460,10 @@ func (c *CI) jobIndexes(b *kv.Batch, j domain.CIJob, old *domain.CIJob) { | |||
| 460 | 460 | } | |
| 461 | 461 | } | |
| 462 | 462 | ||
| 463 | - | // guardRepo makes the batch depend on the repository, like | |
| 464 | - | // MergeRequests.guardRepo, for the writes that create keys under it. | |
| 463 | + | // guardRepo is guardRepoKey on the database of the CI store, for the writes that | |
| 464 | + | // create keys under a repository. | |
| 465 | 465 | func (c *CI) guardRepo(ctx context.Context, b *kv.Batch, repo domain.RepoName) error { | |
| 466 | - | item, ok := c.db.Get(ctx, repoKey(repo)) | |
| 467 | - | if !ok { | |
| 468 | - | return domain.ErrNotFound | |
| 469 | - | } | |
| 470 | - | b.RequireVersion(repoKey(repo), item.Version) | |
| 471 | - | b.Put(repoKey(repo), item.Value) | |
| 472 | - | return nil | |
| 466 | + | return guardRepoKey(ctx, c.db, b, repo) | |
| 473 | 467 | } | |
| 474 | 468 | ||
| 475 | 469 | // stageRecords stages the writes of w. guarded collects the repositories that | |
| @@ -460,16 +460,10 @@ func (c *CI) jobIndexes(b *kv.Batch, j domain.CIJob, old *domain.CIJob) { | |||||
| 460 | } | 460 | } | ||
| 461 | } | 461 | } | ||
| 462 | 462 | ||||
| 463 | - | // guardRepo makes the batch depend on the repository, like | 463 | + | // guardRepo is guardRepoKey on the database of the CI store, for the writes that |
| 464 | - | // MergeRequests.guardRepo, for the writes that create keys under it. | 464 | + | // create keys under a repository. |
| 465 | func (c *CI) guardRepo(ctx context.Context, b *kv.Batch, repo domain.RepoName) error { | 465 | func (c *CI) guardRepo(ctx context.Context, b *kv.Batch, repo domain.RepoName) error { | ||
| 466 | - | item, ok := c.db.Get(ctx, repoKey(repo)) | 466 | + | return guardRepoKey(ctx, c.db, b, repo) |
| 467 | - | if !ok { | |||
| 468 | - | return domain.ErrNotFound | |||
| 469 | - | } | |||
| 470 | - | b.RequireVersion(repoKey(repo), item.Version) | |||
| 471 | - | b.Put(repoKey(repo), item.Value) | |||
| 472 | - | return nil | |||
| 473 | } | 467 | } | ||
| 474 | 468 | ||||
| 475 | // stageRecords stages the writes of w. guarded collects the repositories that | 469 | // stageRecords stages the writes of w. guarded collects the repositories that | ||
M internal/adapters/kvstore/mergerequests.go +2 -19
| @@ -71,26 +71,9 @@ func mergeRequestKeys(ctx context.Context, snap *kv.Snapshot, repo domain.RepoNa | |||
| 71 | 71 | return keys | |
| 72 | 72 | } | |
| 73 | 73 | ||
| 74 | - | // guardRepo makes the batch depend on the repository: it requires the | |
| 75 | - | // repository key to be at the version just read and rewrites it, which gives it | |
| 76 | - | // a new version. Every write that adds a key under the repository's merge | |
| 77 | - | // requests (a request, a comment, a read position) goes through it, so that | |
| 78 | - | // | |
| 79 | - | // - such a write fails with domain.ErrNotFound once the repository is deleted, | |
| 80 | - | // and leaves no key that a repository created again under the name would | |
| 81 | - | // inherit; | |
| 82 | - | // - Store.Delete, which requires the version of the repository key it listed | |
| 83 | - | // the keys at, notices a write that got in after the listing and lists again. | |
| 84 | - | // | |
| 85 | - | // It returns domain.ErrNotFound when the repository does not exist. | |
| 74 | + | // guardRepo is guardRepoKey on the database of the merge requests; see there. | |
| 86 | 75 | func (m *MergeRequests) guardRepo(ctx context.Context, b *kv.Batch, repo domain.RepoName) error { | |
| 87 | - | item, ok := m.db.Get(ctx, repoKey(repo)) | |
| 88 | - | if !ok { | |
| 89 | - | return domain.ErrNotFound | |
| 90 | - | } | |
| 91 | - | b.RequireVersion(repoKey(repo), item.Version) | |
| 92 | - | b.Put(repoKey(repo), item.Value) | |
| 93 | - | return nil | |
| 76 | + | return guardRepoKey(ctx, m.db, b, repo) | |
| 94 | 77 | } | |
| 95 | 78 | ||
| 96 | 79 | // Create takes the next number of the repository from mrseq/<repo> and stores | |
| @@ -71,26 +71,9 @@ func mergeRequestKeys(ctx context.Context, snap *kv.Snapshot, repo domain.RepoNa | |||||
| 71 | return keys | 71 | return keys | ||
| 72 | } | 72 | } | ||
| 73 | 73 | ||||
| 74 | - | // guardRepo makes the batch depend on the repository: it requires the | 74 | + | // guardRepo is guardRepoKey on the database of the merge requests; see there. |
| 75 | - | // repository key to be at the version just read and rewrites it, which gives it | |||
| 76 | - | // a new version. Every write that adds a key under the repository's merge | |||
| 77 | - | // requests (a request, a comment, a read position) goes through it, so that | |||
| 78 | - | // | |||
| 79 | - | // - such a write fails with domain.ErrNotFound once the repository is deleted, | |||
| 80 | - | // and leaves no key that a repository created again under the name would | |||
| 81 | - | // inherit; | |||
| 82 | - | // - Store.Delete, which requires the version of the repository key it listed | |||
| 83 | - | // the keys at, notices a write that got in after the listing and lists again. | |||
| 84 | - | // | |||
| 85 | - | // It returns domain.ErrNotFound when the repository does not exist. | |||
| 86 | func (m *MergeRequests) guardRepo(ctx context.Context, b *kv.Batch, repo domain.RepoName) error { | 75 | func (m *MergeRequests) guardRepo(ctx context.Context, b *kv.Batch, repo domain.RepoName) error { | ||
| 87 | - | item, ok := m.db.Get(ctx, repoKey(repo)) | 76 | + | return guardRepoKey(ctx, m.db, b, repo) |
| 88 | - | if !ok { | |||
| 89 | - | return domain.ErrNotFound | |||
| 90 | - | } | |||
| 91 | - | b.RequireVersion(repoKey(repo), item.Version) | |||
| 92 | - | b.Put(repoKey(repo), item.Value) | |||
| 93 | - | return nil | |||
| 94 | } | 77 | } | ||
| 95 | 78 | ||||
| 96 | // Create takes the next number of the repository from mrseq/<repo> and stores | 79 | // Create takes the next number of the repository from mrseq/<repo> and stores | ||
M internal/adapters/kvstore/repos.go +24 -1
| @@ -90,6 +90,29 @@ func (s *Store) Update(ctx context.Context, repo domain.Repo) error { | |||
| 90 | 90 | return nil | |
| 91 | 91 | } | |
| 92 | 92 | ||
| 93 | + | // guardRepoKey makes the batch depend on the repository: it requires the | |
| 94 | + | // repository key to be at the version just read and rewrites it, which gives it | |
| 95 | + | // a new version. Every write that adds a key under the repository (a merge | |
| 96 | + | // request, a comment, a read position, a CI run, job or check) goes through it, | |
| 97 | + | // so that | |
| 98 | + | // | |
| 99 | + | // - such a write fails with domain.ErrNotFound once the repository is deleted, | |
| 100 | + | // and leaves no key that a repository created again under the name would | |
| 101 | + | // inherit; | |
| 102 | + | // - Store.Delete, which requires the version of the repository key it listed | |
| 103 | + | // the keys at, notices a write that got in after the listing and lists again. | |
| 104 | + | // | |
| 105 | + | // It returns domain.ErrNotFound when the repository does not exist. | |
| 106 | + | func guardRepoKey(ctx context.Context, db *kv.DB, b *kv.Batch, repo domain.RepoName) error { | |
| 107 | + | item, ok := db.Get(ctx, repoKey(repo)) | |
| 108 | + | if !ok { | |
| 109 | + | return domain.ErrNotFound | |
| 110 | + | } | |
| 111 | + | b.RequireVersion(repoKey(repo), item.Version) | |
| 112 | + | b.Put(repoKey(repo), item.Value) | |
| 113 | + | return nil | |
| 114 | + | } | |
| 115 | + | ||
| 93 | 116 | // deleteChunk is how many merge request keys one batch deletes before the | |
| 94 | 117 | // last one, which also removes the repository. | |
| 95 | 118 | const deleteChunk = 1000 | |
| @@ -110,7 +133,7 @@ const deleteTries = 32 | |||
| 110 | 133 | // the last chunk: a delete that fails half way leaves the repository, which | |
| 111 | 134 | // the caller can delete again. The last batch requires the repository key to | |
| 112 | 135 | // be at the version the keys were listed at. Every write of a merge request key | |
| 113 | - | // rewrites the repository key (see MergeRequests.guardRepo), so a write that | |
| 136 | + | // rewrites the repository key (see guardRepoKey), so a write that | |
| 114 | 137 | // got in after the listing makes the batch fail and the keys are listed again, | |
| 115 | 138 | // and a write after the batch finds no repository: no key outlives the | |
| 116 | 139 | // repository to attach itself to a repository created again. | |
| @@ -90,6 +90,29 @@ func (s *Store) Update(ctx context.Context, repo domain.Repo) error { | |||||
| 90 | return nil | 90 | return nil | ||
| 91 | } | 91 | } | ||
| 92 | 92 | ||||
| 93 | + | // guardRepoKey makes the batch depend on the repository: it requires the | |||
| 94 | + | // repository key to be at the version just read and rewrites it, which gives it | |||
| 95 | + | // a new version. Every write that adds a key under the repository (a merge | |||
| 96 | + | // request, a comment, a read position, a CI run, job or check) goes through it, | |||
| 97 | + | // so that | |||
| 98 | + | // | |||
| 99 | + | // - such a write fails with domain.ErrNotFound once the repository is deleted, | |||
| 100 | + | // and leaves no key that a repository created again under the name would | |||
| 101 | + | // inherit; | |||
| 102 | + | // - Store.Delete, which requires the version of the repository key it listed | |||
| 103 | + | // the keys at, notices a write that got in after the listing and lists again. | |||
| 104 | + | // | |||
| 105 | + | // It returns domain.ErrNotFound when the repository does not exist. | |||
| 106 | + | func guardRepoKey(ctx context.Context, db *kv.DB, b *kv.Batch, repo domain.RepoName) error { | |||
| 107 | + | item, ok := db.Get(ctx, repoKey(repo)) | |||
| 108 | + | if !ok { | |||
| 109 | + | return domain.ErrNotFound | |||
| 110 | + | } | |||
| 111 | + | b.RequireVersion(repoKey(repo), item.Version) | |||
| 112 | + | b.Put(repoKey(repo), item.Value) | |||
| 113 | + | return nil | |||
| 114 | + | } | |||
| 115 | + | ||||
| 93 | // deleteChunk is how many merge request keys one batch deletes before the | 116 | // deleteChunk is how many merge request keys one batch deletes before the | ||
| 94 | // last one, which also removes the repository. | 117 | // last one, which also removes the repository. | ||
| 95 | const deleteChunk = 1000 | 118 | const deleteChunk = 1000 | ||
| @@ -110,7 +133,7 @@ const deleteTries = 32 | |||||
| 110 | // the last chunk: a delete that fails half way leaves the repository, which | 133 | // the last chunk: a delete that fails half way leaves the repository, which | ||
| 111 | // the caller can delete again. The last batch requires the repository key to | 134 | // the caller can delete again. The last batch requires the repository key to | ||
| 112 | // be at the version the keys were listed at. Every write of a merge request key | 135 | // be at the version the keys were listed at. Every write of a merge request key | ||
| 113 | - | // rewrites the repository key (see MergeRequests.guardRepo), so a write that | 136 | + | // rewrites the repository key (see guardRepoKey), so a write that |
| 114 | // got in after the listing makes the batch fail and the keys are listed again, | 137 | // got in after the listing makes the batch fail and the keys are listed again, | ||
| 115 | // and a write after the batch finds no repository: no key outlives the | 138 | // and a write after the batch finds no repository: no key outlives the | ||
| 116 | // repository to attach itself to a repository created again. | 139 | // repository to attach itself to a repository created again. | ||