refactor(mr): reconcile mergeability once per repository
- commit
- 1036507b76606d57ce4e83cacd962257f02c8822
- author
- brice · · 2026-10-19 15:57 UTC
- committer
- brice · · 2026-10-19 18:30 UTC
- parent
- 0019f14
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +404 -897
M internal/usecase/export_test.go +4 -1
| @@ -183,10 +183,13 @@ func (s *Mergeability) ScheduledRetries() int { | |||
| 183 | 183 | return len(s.heap) | |
| 184 | 184 | } | |
| 185 | 185 | ||
| 186 | - | // RetryAttempts gives the number of repositories (and the global key) whose | |
| 186 | + | // RetryAttempts gives the number of repositories whose | |
| 187 | 187 | // failed attempts are counted. | |
| 188 | 188 | func (s *Mergeability) RetryAttempts() int { | |
| 189 | 189 | s.retryMu.Lock() | |
| 190 | 190 | defer s.retryMu.Unlock() | |
| 191 | 191 | return len(s.retries) | |
| 192 | 192 | } | |
| 193 | + | ||
| 194 | + | // ReconcileForTest runs a repository pass synchronously without workers. | |
| 195 | + | func (s *Mergeability) ReconcileForTest(repo domain.RepoName) bool { return s.reconcile(repo) } | |
| @@ -183,10 +183,13 @@ func (s *Mergeability) ScheduledRetries() int { | |||||
| 183 | return len(s.heap) | 183 | return len(s.heap) | ||
| 184 | } | 184 | } | ||
| 185 | 185 | ||||
| 186 | - | // RetryAttempts gives the number of repositories (and the global key) whose | 186 | + | // RetryAttempts gives the number of repositories whose |
| 187 | // failed attempts are counted. | 187 | // failed attempts are counted. | ||
| 188 | func (s *Mergeability) RetryAttempts() int { | 188 | func (s *Mergeability) RetryAttempts() int { | ||
| 189 | s.retryMu.Lock() | 189 | s.retryMu.Lock() | ||
| 190 | defer s.retryMu.Unlock() | 190 | defer s.retryMu.Unlock() | ||
| 191 | return len(s.retries) | 191 | return len(s.retries) | ||
| 192 | } | 192 | } | ||
| 193 | + | ||||
| 194 | + | // ReconcileForTest runs a repository pass synchronously without workers. | |||
| 195 | + | func (s *Mergeability) ReconcileForTest(repo domain.RepoName) bool { return s.reconcile(repo) } | |||
M internal/usecase/mergeability.go +219 -358
| @@ -16,31 +16,11 @@ import ( | |||
| 16 | 16 | ||
| 17 | 17 | const ( | |
| 18 | 18 | defaultMergeabilityWorkers = 2 | |
| 19 | - | defaultMergeabilityQueue = 1024 | |
| 20 | - | // mergeabilityAttempts bounds the computations of one check that lose a | |
| 21 | - | // race with a moving branch or a concurrent write; the check is then queued | |
| 22 | - | // again. | |
| 23 | - | mergeabilityAttempts = 3 | |
| 24 | - | // Failed checks (Git or store errors) are retried with an exponential | |
| 25 | - | // backoff, up to mergeabilityRetries times, then dropped until the next | |
| 26 | - | // trigger. | |
| 27 | - | mergeabilityRetries = 5 | |
| 28 | - | mergeabilityRetryInitial = time.Second | |
| 29 | - | mergeabilityRetryMax = time.Minute | |
| 19 | + | mergeabilityRetries = 5 | |
| 20 | + | mergeabilityRetryInitial = time.Second | |
| 21 | + | mergeabilityRetryMax = time.Minute | |
| 30 | 22 | ) | |
| 31 | 23 | ||
| 32 | - | // retryKey names the work a scheduled retry redoes: the rescan of one | |
| 33 | - | // repository, or (all) the rescan of every repository. Retry state is thus | |
| 34 | - | // bounded by the number of repositories plus one. | |
| 35 | - | type retryKey struct { | |
| 36 | - | repo domain.RepoName | |
| 37 | - | all bool | |
| 38 | - | } | |
| 39 | - | ||
| 40 | - | func repoKey(repo domain.RepoName) retryKey { return retryKey{repo: repo} } | |
| 41 | - | ||
| 42 | - | var globalKey = retryKey{all: true} | |
| 43 | - | ||
| 44 | 24 | // mergeBackoff is the delay before retry number attempt (1-based). | |
| 45 | 25 | func mergeBackoff(attempt int) time.Duration { | |
| 46 | 26 | d := mergeabilityRetryInitial | |
| @@ -53,85 +33,90 @@ func mergeBackoff(attempt int) time.Duration { | |||
| 53 | 33 | return d | |
| 54 | 34 | } | |
| 55 | 35 | ||
| 56 | - | // MergeabilityConfig holds what the mergeability service needs. Workers and | |
| 57 | - | // Queue default to 2 and 1024. | |
| 36 | + | // MergeabilityConfig holds the dependencies of the repository reconciler. | |
| 58 | 37 | type MergeabilityConfig struct { | |
| 59 | - | Store ports.MergeRequestStore | |
| 60 | - | Meta ports.RepoMetaStore | |
| 61 | - | Git ports.MergeGit | |
| 62 | - | Bus ports.EventBus | |
| 63 | - | Clock ports.Clock | |
| 64 | - | // Workers is the number of checks that run at once, Queue the number of | |
| 65 | - | // checks that may wait; past it a repository is rescanned. | |
| 66 | - | Workers, Queue int | |
| 67 | - | // After returns a channel that fires after d; it defaults to time.After. | |
| 68 | - | // Tests inject it to drive the retry backoff. | |
| 69 | - | After func(d time.Duration) <-chan time.Time | |
| 38 | + | Store ports.MergeRequestStore | |
| 39 | + | Meta ports.RepoMetaStore | |
| 40 | + | Git ports.MergeGit | |
| 41 | + | Bus ports.EventBus | |
| 42 | + | Clock ports.Clock | |
| 43 | + | Workers int // defaults to 2 | |
| 44 | + | // After is injectable to drive backoff without wall-clock waits in tests. | |
| 45 | + | After func(time.Duration) <-chan time.Time | |
| 70 | 46 | } | |
| 71 | 47 | ||
| 72 | - | // Mergeability keeps up to date whether each open merge request merges with | |
| 73 | - | // its target (see domain.Mergeability). It is independent of CI: it never | |
| 74 | - | // touches refs/pull, workflows or runs. It computes with Git reads only, holds | |
| 75 | - | // no repository lock, and stores its result with a compare-and-swap. | |
| 48 | + | // Mergeability reconciles derived records, with one writer per repository. | |
| 49 | + | // Its work and retry state are bounded by the number of repositories. | |
| 76 | 50 | type Mergeability struct { | |
| 77 | - | cfg MergeabilityConfig | |
| 78 | - | q *mergeQueue | |
| 79 | - | ctx context.Context | |
| 80 | - | cancel context.CancelFunc | |
| 81 | - | wg sync.WaitGroup | |
| 82 | - | ||
| 83 | - | retryMu sync.Mutex | |
| 84 | - | retries map[retryKey]int // attempts so far of the scheduled or failing work | |
| 85 | - | scheduled map[retryKey]bool | |
| 86 | - | heap []retryItem // sorted by due time | |
| 87 | - | retryKick chan struct{} | |
| 88 | - | closed bool | |
| 51 | + | cfg MergeabilityConfig | |
| 52 | + | ctx context.Context | |
| 53 | + | cancel context.CancelFunc | |
| 54 | + | wg sync.WaitGroup | |
| 55 | + | mu sync.Mutex | |
| 56 | + | dirty map[domain.RepoName]bool | |
| 57 | + | order []domain.RepoName | |
| 58 | + | running, again map[domain.RepoName]bool | |
| 59 | + | wake chan struct{} | |
| 60 | + | retryMu sync.Mutex | |
| 61 | + | retries map[domain.RepoName]int | |
| 62 | + | scheduled map[domain.RepoName]bool | |
| 63 | + | heap []retryItem | |
| 64 | + | retryKick chan struct{} | |
| 65 | + | closed bool | |
| 89 | 66 | } | |
| 90 | 67 | ||
| 91 | - | // MergeabilityHook is what the merge request use cases need from the | |
| 92 | - | // mergeability service: queue checks, read results and drop the record of a | |
| 93 | - | // request that is no longer open. | |
| 68 | + | // MergeabilityHook is the unchanged trigger and reader contract. | |
| 94 | 69 | type MergeabilityHook interface { | |
| 95 | 70 | ports.MergeabilityTrigger | |
| 96 | 71 | ports.MergeabilityReader | |
| 97 | - | // OnClosed deletes the record of a merge request that left the open state. | |
| 98 | - | OnClosed(ctx context.Context, mr domain.MergeRequest) | |
| 99 | 72 | } | |
| 100 | 73 | ||
| 101 | 74 | var _ MergeabilityHook = (*Mergeability)(nil) | |
| 102 | 75 | ||
| 103 | - | // NewMergeability builds the service. Call Start to run it, Close to stop it. | |
| 76 | + | // NewMergeability builds a reconciler whose lifetime is controlled by Close. | |
| 104 | 77 | func NewMergeability(ctx context.Context, cfg MergeabilityConfig) *Mergeability { | |
| 105 | 78 | if cfg.Workers <= 0 { | |
| 106 | 79 | cfg.Workers = defaultMergeabilityWorkers | |
| 107 | 80 | } | |
| 108 | - | if cfg.Queue <= 0 { | |
| 109 | - | cfg.Queue = defaultMergeabilityQueue | |
| 110 | - | } | |
| 111 | 81 | if cfg.After == nil { | |
| 112 | 82 | cfg.After = time.After | |
| 113 | 83 | } | |
| 114 | - | // The workers outlive the cancellation of ctx: Close controls their life. | |
| 115 | - | wctx, cancel := context.WithCancel(context.WithoutCancel(ctx)) | |
| 116 | - | return &Mergeability{ | |
| 117 | - | cfg: cfg, q: newMergeQueue(cfg.Queue), ctx: wctx, cancel: cancel, | |
| 118 | - | retries: map[retryKey]int{}, scheduled: map[retryKey]bool{}, retryKick: make(chan struct{}, 1), | |
| 119 | - | } | |
| 84 | + | ctx, cancel := context.WithCancel(context.WithoutCancel(ctx)) | |
| 85 | + | return &Mergeability{cfg: cfg, ctx: ctx, cancel: cancel, | |
| 86 | + | dirty: map[domain.RepoName]bool{}, running: map[domain.RepoName]bool{}, again: map[domain.RepoName]bool{}, wake: make(chan struct{}, 1), | |
| 87 | + | retries: map[domain.RepoName]int{}, scheduled: map[domain.RepoName]bool{}, retryKick: make(chan struct{}, 1)} | |
| 120 | 88 | } | |
| 121 | 89 | ||
| 122 | - | // Start rescans the open merge requests of every repository (the records may | |
| 123 | - | // be missing or stale after a restart) and starts the workers. | |
| 90 | + | // Start starts workers and discovers repositories, retrying failed discovery. | |
| 124 | 91 | func (s *Mergeability) Start() { | |
| 125 | - | s.wg.Add(1) | |
| 92 | + | s.wg.Add(2 + s.cfg.Workers) | |
| 126 | 93 | go s.scheduler() | |
| 127 | - | s.rescanAll() | |
| 94 | + | go func() { | |
| 95 | + | defer s.wg.Done() | |
| 96 | + | for attempt := 0; attempt <= mergeabilityRetries; attempt++ { | |
| 97 | + | repos, err := s.cfg.Meta.List(s.ctx) | |
| 98 | + | if err == nil { | |
| 99 | + | for _, repo := range repos { | |
| 100 | + | s.markDirty(repo.Name) | |
| 101 | + | } | |
| 102 | + | return | |
| 103 | + | } | |
| 104 | + | if attempt == mergeabilityRetries { | |
| 105 | + | return | |
| 106 | + | } | |
| 107 | + | select { | |
| 108 | + | case <-s.ctx.Done(): | |
| 109 | + | return | |
| 110 | + | case <-s.cfg.After(mergeBackoff(attempt + 1)): | |
| 111 | + | } | |
| 112 | + | } | |
| 113 | + | }() | |
| 128 | 114 | for range s.cfg.Workers { | |
| 129 | - | s.wg.Add(1) | |
| 130 | 115 | go s.loop() | |
| 131 | 116 | } | |
| 132 | 117 | } | |
| 133 | 118 | ||
| 134 | - | // Close stops the workers and waits for them. | |
| 119 | + | // Close cancels workers and pending retry waits, then waits for them. | |
| 135 | 120 | func (s *Mergeability) Close(_ context.Context) { | |
| 136 | 121 | s.retryMu.Lock() | |
| 137 | 122 | s.closed = true | |
| @@ -140,16 +125,160 @@ func (s *Mergeability) Close(_ context.Context) { | |||
| 140 | 125 | s.wg.Wait() | |
| 141 | 126 | } | |
| 142 | 127 | ||
| 128 | + | func (s *Mergeability) signal() { | |
| 129 | + | select { | |
| 130 | + | case s.wake <- struct{}{}: | |
| 131 | + | default: | |
| 132 | + | } | |
| 133 | + | } | |
| 134 | + | ||
| 135 | + | func (s *Mergeability) markDirty(repo domain.RepoName) { | |
| 136 | + | s.mu.Lock() | |
| 137 | + | if s.running[repo] { | |
| 138 | + | s.again[repo] = true | |
| 139 | + | } else if !s.dirty[repo] { | |
| 140 | + | s.dirty[repo] = true | |
| 141 | + | s.order = append(s.order, repo) | |
| 142 | + | } | |
| 143 | + | s.mu.Unlock() | |
| 144 | + | s.signal() | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | func (s *Mergeability) next() (domain.RepoName, bool) { | |
| 148 | + | s.mu.Lock() | |
| 149 | + | defer s.mu.Unlock() | |
| 150 | + | if len(s.order) == 0 { | |
| 151 | + | return domain.RepoName{}, false | |
| 152 | + | } | |
| 153 | + | repo := s.order[0] | |
| 154 | + | s.order = s.order[1:] | |
| 155 | + | delete(s.dirty, repo) | |
| 156 | + | s.running[repo] = true | |
| 157 | + | if len(s.order) > 0 { | |
| 158 | + | s.signal() | |
| 159 | + | } | |
| 160 | + | return repo, true | |
| 161 | + | } | |
| 162 | + | ||
| 163 | + | func (s *Mergeability) loop() { | |
| 164 | + | defer s.wg.Done() | |
| 165 | + | for s.ctx.Err() == nil { | |
| 166 | + | repo, found := s.next() | |
| 167 | + | if !found { | |
| 168 | + | select { | |
| 169 | + | case <-s.ctx.Done(): | |
| 170 | + | return | |
| 171 | + | case <-s.wake: | |
| 172 | + | } | |
| 173 | + | continue | |
| 174 | + | } | |
| 175 | + | if s.reconcile(repo) { | |
| 176 | + | s.retryDone(repo) | |
| 177 | + | } else { | |
| 178 | + | s.retry(repo) | |
| 179 | + | } | |
| 180 | + | s.mu.Lock() | |
| 181 | + | delete(s.running, repo) | |
| 182 | + | if s.again[repo] { | |
| 183 | + | delete(s.again, repo) | |
| 184 | + | s.dirty[repo] = true | |
| 185 | + | s.order = append(s.order, repo) | |
| 186 | + | s.signal() | |
| 187 | + | } | |
| 188 | + | s.mu.Unlock() | |
| 189 | + | } | |
| 190 | + | } | |
| 191 | + | ||
| 192 | + | // OnOpened implements ports.MergeabilityTrigger. | |
| 193 | + | func (s *Mergeability) OnOpened(_ context.Context, mr domain.MergeRequest) { s.markDirty(mr.Repo) } | |
| 194 | + | ||
| 195 | + | // OnBranchesMoved dirties the repository for any branch update. | |
| 196 | + | func (s *Mergeability) OnBranchesMoved(_ context.Context, repo domain.RepoName, updates []domain.RefUpdate) { | |
| 197 | + | for _, update := range updates { | |
| 198 | + | if strings.HasPrefix(update.Ref, "refs/heads/") { | |
| 199 | + | s.markDirty(repo) | |
| 200 | + | return | |
| 201 | + | } | |
| 202 | + | } | |
| 203 | + | } | |
| 204 | + | ||
| 205 | + | // Read returns a current record, or checking and requests reconciliation. | |
| 206 | + | func (s *Mergeability) Read(ctx context.Context, mr domain.MergeRequest, targetTip domain.ObjectID) domain.Mergeability { | |
| 207 | + | if !mr.HeadID.IsZero() && !targetTip.IsZero() { | |
| 208 | + | rec, err := s.cfg.Store.GetMergeability(ctx, mr.Repo, mr.Number) | |
| 209 | + | if err == nil && rec.IsFor(targetTip, mr.HeadID) { | |
| 210 | + | return rec | |
| 211 | + | } | |
| 212 | + | } | |
| 213 | + | s.markDirty(mr.Repo) | |
| 214 | + | return domain.Mergeability{Repo: mr.Repo, Number: mr.Number, State: domain.MergeabilityChecking} | |
| 215 | + | } | |
| 216 | + | ||
| 217 | + | // reconcile settles each open request for the tips read in this pass. A push | |
| 218 | + | // during computation requests another pass; old-tip records are harmless. | |
| 219 | + | func (s *Mergeability) reconcile(repo domain.RepoName) bool { | |
| 220 | + | ctx := s.ctx | |
| 221 | + | meta, err := s.cfg.Meta.Get(ctx, repo) | |
| 222 | + | if errors.Is(err, domain.ErrNotFound) { | |
| 223 | + | return true | |
| 224 | + | } | |
| 225 | + | if err != nil { | |
| 226 | + | return false | |
| 227 | + | } | |
| 228 | + | mrs, err := s.cfg.Store.ListOpen(ctx, repo) | |
| 229 | + | if err != nil { | |
| 230 | + | return false | |
| 231 | + | } | |
| 232 | + | ok := true | |
| 233 | + | for _, mr := range mrs { | |
| 234 | + | if ctx.Err() != nil { | |
| 235 | + | return true | |
| 236 | + | } | |
| 237 | + | head, tip, found, transient := s.tips(ctx, mr) | |
| 238 | + | if !found { | |
| 239 | + | ok = ok && !transient | |
| 240 | + | continue | |
| 241 | + | } | |
| 242 | + | prev, err := s.cfg.Store.GetMergeability(ctx, repo, mr.Number) | |
| 243 | + | if err != nil && !errors.Is(err, domain.ErrNotFound) { | |
| 244 | + | ok = false | |
| 245 | + | continue | |
| 246 | + | } | |
| 247 | + | if prev.IsFor(tip, head) { | |
| 248 | + | continue | |
| 249 | + | } | |
| 250 | + | res, err := s.compute(ctx, repo, mr.Number, tip, head) | |
| 251 | + | if err != nil { | |
| 252 | + | ok = false | |
| 253 | + | continue | |
| 254 | + | } | |
| 255 | + | res.Version = prev.Version | |
| 256 | + | err = s.cfg.Store.PutMergeability(ctx, res) | |
| 257 | + | if errors.Is(err, domain.ErrNotFound) { | |
| 258 | + | continue | |
| 259 | + | } | |
| 260 | + | if err != nil { | |
| 261 | + | ok = false | |
| 262 | + | continue | |
| 263 | + | } | |
| 264 | + | s.cfg.Bus.Publish(ctx, domain.MergeRequestMergeability{ | |
| 265 | + | Repo: repo, Number: mr.Number, Mergeability: res, At: res.CheckedAt, | |
| 266 | + | Visibility: meta.Visibility, CreatedAt: meta.CreatedAt, | |
| 267 | + | }) | |
| 268 | + | } | |
| 269 | + | return ok | |
| 270 | + | } | |
| 271 | + | ||
| 143 | 272 | // retryItem is one scheduled retry. | |
| 144 | 273 | type retryItem struct { | |
| 145 | 274 | due time.Time | |
| 146 | - | key retryKey | |
| 275 | + | key domain.RepoName | |
| 147 | 276 | } | |
| 148 | 277 | ||
| 149 | 278 | // retry schedules a retry of key after a bounded exponential backoff. It is | |
| 150 | 279 | // deduplicated per key, gives up after mergeabilityRetries attempts and stops | |
| 151 | 280 | // on Close. All retries share one scheduler goroutine. | |
| 152 | - | func (s *Mergeability) retry(key retryKey) { | |
| 281 | + | func (s *Mergeability) retry(key domain.RepoName) { | |
| 153 | 282 | s.retryMu.Lock() | |
| 154 | 283 | defer s.retryMu.Unlock() | |
| 155 | 284 | if s.closed || s.scheduled[key] { | |
| @@ -220,17 +349,17 @@ func (s *Mergeability) scheduler() { | |||
| 220 | 349 | if s.ctx.Err() != nil { | |
| 221 | 350 | return | |
| 222 | 351 | } | |
| 223 | - | s.fire(k) | |
| 352 | + | s.markDirty(k) | |
| 224 | 353 | } | |
| 225 | 354 | } | |
| 226 | 355 | } | |
| 227 | 356 | ||
| 228 | 357 | // popDue removes the waited item and the ones due as well. | |
| 229 | - | func (s *Mergeability) popDue(waited retryKey) []retryKey { | |
| 358 | + | func (s *Mergeability) popDue(waited domain.RepoName) []domain.RepoName { | |
| 230 | 359 | s.retryMu.Lock() | |
| 231 | 360 | defer s.retryMu.Unlock() | |
| 232 | 361 | now := s.cfg.Clock.Now() | |
| 233 | - | var out []retryKey | |
| 362 | + | var out []domain.RepoName | |
| 234 | 363 | s.heap = slices.DeleteFunc(s.heap, func(it retryItem) bool { | |
| 235 | 364 | if it.key != waited && it.due.After(now) { | |
| 236 | 365 | return false | |
| @@ -242,260 +371,17 @@ func (s *Mergeability) popDue(waited retryKey) []retryKey { | |||
| 242 | 371 | return out | |
| 243 | 372 | } | |
| 244 | 373 | ||
| 245 | - | // fire puts the work of a due retry back in the normal queue, so that the | |
| 246 | - | // worker limits apply. | |
| 247 | - | func (s *Mergeability) fire(k retryKey) { | |
| 248 | - | if k.all { | |
| 249 | - | s.rescanAll() | |
| 250 | - | return | |
| 251 | - | } | |
| 252 | - | s.q.markDirty(k.repo) | |
| 253 | - | } | |
| 254 | - | ||
| 255 | 374 | // retryDone forgets the failures of key after a success. | |
| 256 | - | func (s *Mergeability) retryDone(key retryKey) { | |
| 375 | + | func (s *Mergeability) retryDone(key domain.RepoName) { | |
| 257 | 376 | s.retryMu.Lock() | |
| 258 | 377 | delete(s.retries, key) | |
| 259 | - | s.retryMu.Unlock() | |
| 260 | - | } | |
| 261 | - | ||
| 262 | - | func (s *Mergeability) rescanAll() { | |
| 263 | - | repos, err := s.cfg.Meta.List(s.ctx) | |
| 264 | - | if err != nil { | |
| 265 | - | ctxLog(s.ctx).Error().Err(err).Msg("mergeability: list the repositories to rescan") | |
| 266 | - | s.retry(globalKey) | |
| 267 | - | return | |
| 268 | - | } | |
| 269 | - | s.retryDone(globalKey) | |
| 270 | - | for _, r := range repos { | |
| 271 | - | n, err := s.cfg.Store.CountOpen(s.ctx, r.Name) | |
| 272 | - | if err != nil { | |
| 273 | - | ctxLog(s.ctx).Error().Err(err).Str("repo", r.Name.String()).Msg("mergeability: count the open merge requests") | |
| 274 | - | s.q.markDirty(r.Name) | |
| 275 | - | continue | |
| 276 | - | } | |
| 277 | - | if n > 0 { | |
| 278 | - | s.q.markDirty(r.Name) | |
| 279 | - | } | |
| 378 | + | delete(s.scheduled, key) | |
| 379 | + | s.heap = slices.DeleteFunc(s.heap, func(it retryItem) bool { return it.key == key }) | |
| 380 | + | select { | |
| 381 | + | case s.retryKick <- struct{}{}: | |
| 382 | + | default: | |
| 280 | 383 | } | |
| 281 | - | } | |
| 282 | - | ||
| 283 | - | // OnOpened implements ports.MergeabilityTrigger. | |
| 284 | - | func (s *Mergeability) OnOpened(_ context.Context, mr domain.MergeRequest) { | |
| 285 | - | s.q.add(mergeKey{mr.Repo, mr.Number}) | |
| 286 | - | } | |
| 287 | - | ||
| 288 | - | // OnClosed deletes the record of a merge request that is no longer open. The | |
| 289 | - | // store already drops it when the request leaves the open state; this makes it | |
| 290 | - | // explicit and is idempotent. | |
| 291 | - | func (s *Mergeability) OnClosed(ctx context.Context, mr domain.MergeRequest) { | |
| 292 | - | if err := s.cfg.Store.DeleteMergeability(context.WithoutCancel(ctx), mr.Repo, mr.Number); err != nil { | |
| 293 | - | ctxLog(ctx).Error().Err(err).Str("repo", mr.Repo.String()).Int64("number", mr.Number).Msg("mergeability: delete the record of a closed merge request") | |
| 294 | - | } | |
| 295 | - | s.q.takeSeen(mergeKey{mr.Repo, mr.Number}) | |
| 296 | - | } | |
| 297 | - | ||
| 298 | - | // OnBranchesMoved implements ports.MergeabilityTrigger: the open merge | |
| 299 | - | // requests whose source or target is one of the branches are queued. Past the | |
| 300 | - | // bound, or when the lookup fails, the repository is rescanned. | |
| 301 | - | func (s *Mergeability) OnBranchesMoved(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate) { | |
| 302 | - | var branches []string | |
| 303 | - | for _, u := range updates { | |
| 304 | - | if b, ok := strings.CutPrefix(u.Ref, "refs/heads/"); ok && !slices.Contains(branches, b) { | |
| 305 | - | branches = append(branches, b) | |
| 306 | - | } | |
| 307 | - | } | |
| 308 | - | if len(branches) == 0 { | |
| 309 | - | return | |
| 310 | - | } | |
| 311 | - | lookups := []func(context.Context, domain.RepoName, []string, int) ([]int64, bool, error){ | |
| 312 | - | s.cfg.Store.OpenBySource, s.cfg.Store.OpenByTarget, | |
| 313 | - | } | |
| 314 | - | for _, lookup := range lookups { | |
| 315 | - | numbers, truncated, err := lookup(ctx, repo, branches, s.cfg.Queue) | |
| 316 | - | if err != nil || truncated { | |
| 317 | - | if err != nil { | |
| 318 | - | ctxLog(ctx).Error().Err(err).Str("repo", repo.String()).Msg("mergeability: look up the merge requests of moved branches: the repository is rescanned") | |
| 319 | - | } | |
| 320 | - | s.q.markDirty(repo) | |
| 321 | - | } | |
| 322 | - | for _, n := range numbers { | |
| 323 | - | s.q.add(mergeKey{repo, n}) | |
| 324 | - | } | |
| 325 | - | } | |
| 326 | - | } | |
| 327 | - | ||
| 328 | - | // Read implements ports.MergeabilityReader. | |
| 329 | - | func (s *Mergeability) Read(ctx context.Context, mr domain.MergeRequest, targetTip domain.ObjectID) domain.Mergeability { | |
| 330 | - | checking := domain.Mergeability{Repo: mr.Repo, Number: mr.Number, State: domain.MergeabilityChecking} | |
| 331 | - | if mr.HeadID.IsZero() || targetTip.IsZero() { | |
| 332 | - | return checking // a branch is gone: there is nothing to check | |
| 333 | - | } | |
| 334 | - | rec, err := s.cfg.Store.GetMergeability(ctx, mr.Repo, mr.Number) | |
| 335 | - | if err == nil && rec.IsFor(targetTip, mr.HeadID) { | |
| 336 | - | return rec | |
| 337 | - | } | |
| 338 | - | k := mergeKey{mr.Repo, mr.Number} | |
| 339 | - | s.q.markSeen(k) | |
| 340 | - | s.q.add(k) | |
| 341 | - | return checking | |
| 342 | - | } | |
| 343 | - | ||
| 344 | - | func (s *Mergeability) loop() { | |
| 345 | - | defer s.wg.Done() | |
| 346 | - | for { | |
| 347 | - | key, dirty := s.q.next() | |
| 348 | - | switch { | |
| 349 | - | case key != nil: | |
| 350 | - | if !s.runKey(*key) { | |
| 351 | - | s.retry(repoKey(key.repo)) | |
| 352 | - | } | |
| 353 | - | case dirty != nil: | |
| 354 | - | s.rescan(*dirty) | |
| 355 | - | default: | |
| 356 | - | select { | |
| 357 | - | case <-s.ctx.Done(): | |
| 358 | - | return | |
| 359 | - | case <-s.q.wake: | |
| 360 | - | } | |
| 361 | - | } | |
| 362 | - | if s.ctx.Err() != nil { | |
| 363 | - | return | |
| 364 | - | } | |
| 365 | - | } | |
| 366 | - | } | |
| 367 | - | ||
| 368 | - | // runKey checks k, which is marked running, as often as it was queued again | |
| 369 | - | // meanwhile. It reports false when a check failed transiently; the caller | |
| 370 | - | // then retries the repository. | |
| 371 | - | func (s *Mergeability) runKey(k mergeKey) bool { | |
| 372 | - | ok := true | |
| 373 | - | for { | |
| 374 | - | ok = s.check(k) && ok | |
| 375 | - | if !s.q.finish(k) || s.ctx.Err() != nil { | |
| 376 | - | return ok | |
| 377 | - | } | |
| 378 | - | } | |
| 379 | - | } | |
| 380 | - | ||
| 381 | - | // rescan checks every open merge request of a repository, here and not through | |
| 382 | - | // the bounded queue, so that a rescan never marks its repository dirty again. | |
| 383 | - | // It succeeds, and resets the retry budget of the repository, only if the | |
| 384 | - | // listing and every check succeeded; otherwise the repository is retried once. | |
| 385 | - | func (s *Mergeability) rescan(repo domain.RepoName) { | |
| 386 | - | mrs, err := s.cfg.Store.ListOpen(s.ctx, repo) | |
| 387 | - | if err != nil { | |
| 388 | - | ctxLog(s.ctx).Error().Err(err).Str("repo", repo.String()).Msg("mergeability: list the open merge requests to rescan") | |
| 389 | - | s.retry(repoKey(repo)) | |
| 390 | - | return | |
| 391 | - | } | |
| 392 | - | ok := true | |
| 393 | - | for _, mr := range mrs { | |
| 394 | - | if s.ctx.Err() != nil { | |
| 395 | - | return | |
| 396 | - | } | |
| 397 | - | k := mergeKey{repo, mr.Number} | |
| 398 | - | if s.q.tryRun(k) { | |
| 399 | - | ok = s.runKey(k) && ok | |
| 400 | - | } | |
| 401 | - | } | |
| 402 | - | if ok { | |
| 403 | - | s.retryDone(repoKey(repo)) | |
| 404 | - | } else { | |
| 405 | - | s.retry(repoKey(repo)) | |
| 406 | - | } | |
| 407 | - | } | |
| 408 | - | ||
| 409 | - | // Check computes the mergeability of a merge request now, on the calling | |
| 410 | - | // goroutine, as a worker does. It is for tests; the server never calls it. | |
| 411 | - | func (s *Mergeability) Check(repo domain.RepoName, number int64) { | |
| 412 | - | k := mergeKey{repo, number} | |
| 413 | - | if s.q.tryRun(k) && !s.runKey(k) { | |
| 414 | - | s.retry(repoKey(k.repo)) | |
| 415 | - | } | |
| 416 | - | } | |
| 417 | - | ||
| 418 | - | // check brings the record of k up to date. It reports false when a transient | |
| 419 | - | // error (store, Git) dropped the work, which is then retried. | |
| 420 | - | func (s *Mergeability) check(k mergeKey) (done bool) { | |
| 421 | - | ctx := s.ctx | |
| 422 | - | log := ctxLog(ctx).With().Str("repo", k.repo.String()).Int64("number", k.number).Logger() | |
| 423 | - | for range mergeabilityAttempts { | |
| 424 | - | mr, err := s.cfg.Store.Get(ctx, k.repo, k.number) | |
| 425 | - | if err != nil { | |
| 426 | - | if errors.Is(err, domain.ErrNotFound) { | |
| 427 | - | s.q.takeSeen(k) | |
| 428 | - | return true | |
| 429 | - | } | |
| 430 | - | if ctx.Err() != nil { | |
| 431 | - | return true | |
| 432 | - | } | |
| 433 | - | log.Error().Err(err).Msg("mergeability: read the merge request") | |
| 434 | - | return false | |
| 435 | - | } | |
| 436 | - | if !mr.IsOpen() { | |
| 437 | - | s.q.takeSeen(k) | |
| 438 | - | return true // the store dropped the record when it left the open state | |
| 439 | - | } | |
| 440 | - | head, tip, ok, transient := s.tips(ctx, mr) | |
| 441 | - | if !ok { | |
| 442 | - | return !transient | |
| 443 | - | } | |
| 444 | - | prev, err := s.cfg.Store.GetMergeability(ctx, k.repo, k.number) | |
| 445 | - | if err != nil && !errors.Is(err, domain.ErrNotFound) { | |
| 446 | - | if ctx.Err() != nil { | |
| 447 | - | return true | |
| 448 | - | } | |
| 449 | - | log.Error().Err(err).Msg("mergeability: read the record") | |
| 450 | - | return false | |
| 451 | - | } | |
| 452 | - | if prev.IsFor(tip, head) { | |
| 453 | - | if s.q.takeSeen(k) && s.publish(ctx, prev) != nil { // a reader saw "checking" before this record | |
| 454 | - | s.q.markSeen(k) | |
| 455 | - | return false | |
| 456 | - | } | |
| 457 | - | return true | |
| 458 | - | } | |
| 459 | - | res, err := s.compute(ctx, k, tip, head) | |
| 460 | - | if err != nil { | |
| 461 | - | if ctx.Err() != nil { | |
| 462 | - | return true | |
| 463 | - | } | |
| 464 | - | log.Error().Err(err).Msg("mergeability: compute") | |
| 465 | - | return false | |
| 466 | - | } | |
| 467 | - | // The branches may have moved during the computation: the result is | |
| 468 | - | // then for tips nobody asks about, and is dropped. | |
| 469 | - | if h2, t2, ok, transient := s.tips(ctx, mr); !ok { | |
| 470 | - | return !transient | |
| 471 | - | } else if h2 != head || t2 != tip { | |
| 472 | - | continue | |
| 473 | - | } | |
| 474 | - | res.Version = prev.Version | |
| 475 | - | switch err := s.cfg.Store.PutMergeability(ctx, res); { | |
| 476 | - | case errors.Is(err, domain.ErrConflict): | |
| 477 | - | continue | |
| 478 | - | case errors.Is(err, domain.ErrNotFound): | |
| 479 | - | s.q.takeSeen(k) | |
| 480 | - | return true // closed meanwhile | |
| 481 | - | case err != nil: | |
| 482 | - | if ctx.Err() != nil { | |
| 483 | - | return true | |
| 484 | - | } | |
| 485 | - | log.Error().Err(err).Msg("mergeability: store the record") | |
| 486 | - | return false | |
| 487 | - | } | |
| 488 | - | seen := s.q.takeSeen(k) | |
| 489 | - | if seen || prev.State == "" || changedMergeability(prev, res) { | |
| 490 | - | if s.publish(ctx, res) != nil { | |
| 491 | - | s.q.markSeen(k) // the next check publishes it | |
| 492 | - | return false | |
| 493 | - | } | |
| 494 | - | } | |
| 495 | - | return true | |
| 496 | - | } | |
| 497 | - | s.q.add(k) // kept losing races: try again later | |
| 498 | - | return true | |
| 384 | + | s.retryMu.Unlock() | |
| 499 | 385 | } | |
| 500 | 386 | ||
| 501 | 387 | // tips reads the heads of the source and the target of mr. ok is false when | |
| @@ -512,7 +398,6 @@ func (s *Mergeability) tips(ctx context.Context, mr domain.MergeRequest) (head, | |||
| 512 | 398 | } | |
| 513 | 399 | transient = ctx.Err() == nil | |
| 514 | 400 | } | |
| 515 | - | s.q.takeSeen(mergeKey{mr.Repo, mr.Number}) | |
| 516 | 401 | return domain.ObjectID{}, domain.ObjectID{}, false, transient | |
| 517 | 402 | case herr != nil || terr != nil: | |
| 518 | 403 | if ctx.Err() != nil { | |
| @@ -525,12 +410,12 @@ func (s *Mergeability) tips(ctx context.Context, mr domain.MergeRequest) (head, | |||
| 525 | 410 | } | |
| 526 | 411 | ||
| 527 | 412 | // compute runs the Git reads that decide the state of head against tip. | |
| 528 | - | func (s *Mergeability) compute(ctx context.Context, k mergeKey, tip, head domain.ObjectID) (domain.Mergeability, error) { | |
| 529 | - | m := domain.Mergeability{Repo: k.repo, Number: k.number, TargetID: tip, HeadID: head, CheckedAt: s.cfg.Clock.Now().UTC()} | |
| 413 | + | func (s *Mergeability) compute(ctx context.Context, repo domain.RepoName, number int64, tip, head domain.ObjectID) (domain.Mergeability, error) { | |
| 414 | + | m := domain.Mergeability{Repo: repo, Number: number, TargetID: tip, HeadID: head, CheckedAt: s.cfg.Clock.Now().UTC()} | |
| 530 | 415 | ancestor := tip == head | |
| 531 | 416 | if !ancestor { | |
| 532 | 417 | var err error | |
| 533 | - | if ancestor, err = s.cfg.Git.IsAncestor(ctx, k.repo, tip, head); err != nil { | |
| 418 | + | if ancestor, err = s.cfg.Git.IsAncestor(ctx, repo, tip, head); err != nil { | |
| 534 | 419 | return m, err | |
| 535 | 420 | } | |
| 536 | 421 | } | |
| @@ -538,7 +423,7 @@ func (s *Mergeability) compute(ctx context.Context, k mergeKey, tip, head domain | |||
| 538 | 423 | m.State = domain.MergeabilityUpToDate | |
| 539 | 424 | return m, nil | |
| 540 | 425 | } | |
| 541 | - | tree, err := s.cfg.Git.MergeTree(ctx, k.repo, tip, head) | |
| 426 | + | tree, err := s.cfg.Git.MergeTree(ctx, repo, tip, head) | |
| 542 | 427 | if err != nil { | |
| 543 | 428 | return m, err | |
| 544 | 429 | } | |
| @@ -549,34 +434,10 @@ func (s *Mergeability) compute(ctx context.Context, k mergeKey, tip, head domain | |||
| 549 | 434 | return m, nil | |
| 550 | 435 | } | |
| 551 | 436 | m.State = domain.MergeabilityBehind | |
| 552 | - | n, err := s.cfg.Git.CountBehind(ctx, k.repo, head, tip, domain.BehindLimit) | |
| 437 | + | n, err := s.cfg.Git.CountBehind(ctx, repo, head, tip, domain.BehindLimit) | |
| 553 | 438 | if err != nil { | |
| 554 | 439 | return m, err | |
| 555 | 440 | } | |
| 556 | 441 | m.Behind, m.BehindCapped = domain.BoundBehind(n) | |
| 557 | 442 | return m, nil | |
| 558 | 443 | } | |
| 559 | - | ||
| 560 | - | // changedMergeability reports whether what a reader shows differs. | |
| 561 | - | func changedMergeability(a, b domain.Mergeability) bool { | |
| 562 | - | return a.State != b.State || a.Behind != b.Behind || a.BehindCapped != b.BehindCapped || | |
| 563 | - | a.ConflictPathsTruncated != b.ConflictPathsTruncated || !slices.Equal(a.ConflictPaths, b.ConflictPaths) | |
| 564 | - | } | |
| 565 | - | ||
| 566 | - | // publish sends the result on the bus. It fails on a transient error; a | |
| 567 | - | // repository that is gone is terminal and not an error. | |
| 568 | - | func (s *Mergeability) publish(ctx context.Context, m domain.Mergeability) error { | |
| 569 | - | repo, err := s.cfg.Meta.Get(ctx, m.Repo) | |
| 570 | - | if err != nil { | |
| 571 | - | if errors.Is(err, domain.ErrNotFound) || ctx.Err() != nil { | |
| 572 | - | return nil | |
| 573 | - | } | |
| 574 | - | ctxLog(ctx).Error().Err(err).Str("repo", m.Repo.String()).Msg("mergeability: read the repository to publish a result") | |
| 575 | - | return err | |
| 576 | - | } | |
| 577 | - | s.cfg.Bus.Publish(ctx, domain.MergeRequestMergeability{ | |
| 578 | - | Repo: m.Repo, Number: m.Number, Mergeability: m, At: m.CheckedAt, | |
| 579 | - | Visibility: repo.Visibility, CreatedAt: repo.CreatedAt, | |
| 580 | - | }) | |
| 581 | - | return nil | |
| 582 | - | } | |
| @@ -16,31 +16,11 @@ import ( | |||||
| 16 | 16 | ||||
| 17 | const ( | 17 | const ( | ||
| 18 | defaultMergeabilityWorkers = 2 | 18 | defaultMergeabilityWorkers = 2 | ||
| 19 | - | defaultMergeabilityQueue = 1024 | 19 | + | mergeabilityRetries = 5 |
| 20 | - | // mergeabilityAttempts bounds the computations of one check that lose a | 20 | + | mergeabilityRetryInitial = time.Second |
| 21 | - | // race with a moving branch or a concurrent write; the check is then queued | 21 | + | mergeabilityRetryMax = time.Minute |
| 22 | - | // again. | |||
| 23 | - | mergeabilityAttempts = 3 | |||
| 24 | - | // Failed checks (Git or store errors) are retried with an exponential | |||
| 25 | - | // backoff, up to mergeabilityRetries times, then dropped until the next | |||
| 26 | - | // trigger. | |||
| 27 | - | mergeabilityRetries = 5 | |||
| 28 | - | mergeabilityRetryInitial = time.Second | |||
| 29 | - | mergeabilityRetryMax = time.Minute | |||
| 30 | ) | 22 | ) | ||
| 31 | 23 | ||||
| 32 | - | // retryKey names the work a scheduled retry redoes: the rescan of one | |||
| 33 | - | // repository, or (all) the rescan of every repository. Retry state is thus | |||
| 34 | - | // bounded by the number of repositories plus one. | |||
| 35 | - | type retryKey struct { | |||
| 36 | - | repo domain.RepoName | |||
| 37 | - | all bool | |||
| 38 | - | } | |||
| 39 | - | ||||
| 40 | - | func repoKey(repo domain.RepoName) retryKey { return retryKey{repo: repo} } | |||
| 41 | - | ||||
| 42 | - | var globalKey = retryKey{all: true} | |||
| 43 | - | ||||
| 44 | // mergeBackoff is the delay before retry number attempt (1-based). | 24 | // mergeBackoff is the delay before retry number attempt (1-based). | ||
| 45 | func mergeBackoff(attempt int) time.Duration { | 25 | func mergeBackoff(attempt int) time.Duration { | ||
| 46 | d := mergeabilityRetryInitial | 26 | d := mergeabilityRetryInitial | ||
| @@ -53,85 +33,90 @@ func mergeBackoff(attempt int) time.Duration { | |||||
| 53 | return d | 33 | return d | ||
| 54 | } | 34 | } | ||
| 55 | 35 | ||||
| 56 | - | // MergeabilityConfig holds what the mergeability service needs. Workers and | 36 | + | // MergeabilityConfig holds the dependencies of the repository reconciler. |
| 57 | - | // Queue default to 2 and 1024. | |||
| 58 | type MergeabilityConfig struct { | 37 | type MergeabilityConfig struct { | ||
| 59 | - | Store ports.MergeRequestStore | 38 | + | Store ports.MergeRequestStore |
| 60 | - | Meta ports.RepoMetaStore | 39 | + | Meta ports.RepoMetaStore |
| 61 | - | Git ports.MergeGit | 40 | + | Git ports.MergeGit |
| 62 | - | Bus ports.EventBus | 41 | + | Bus ports.EventBus |
| 63 | - | Clock ports.Clock | 42 | + | Clock ports.Clock |
| 64 | - | // Workers is the number of checks that run at once, Queue the number of | 43 | + | Workers int // defaults to 2 |
| 65 | - | // checks that may wait; past it a repository is rescanned. | 44 | + | // After is injectable to drive backoff without wall-clock waits in tests. |
| 66 | - | Workers, Queue int | 45 | + | After func(time.Duration) <-chan time.Time |
| 67 | - | // After returns a channel that fires after d; it defaults to time.After. | |||
| 68 | - | // Tests inject it to drive the retry backoff. | |||
| 69 | - | After func(d time.Duration) <-chan time.Time | |||
| 70 | } | 46 | } | ||
| 71 | 47 | ||||
| 72 | - | // Mergeability keeps up to date whether each open merge request merges with | 48 | + | // Mergeability reconciles derived records, with one writer per repository. |
| 73 | - | // its target (see domain.Mergeability). It is independent of CI: it never | 49 | + | // Its work and retry state are bounded by the number of repositories. |
| 74 | - | // touches refs/pull, workflows or runs. It computes with Git reads only, holds | |||
| 75 | - | // no repository lock, and stores its result with a compare-and-swap. | |||
| 76 | type Mergeability struct { | 50 | type Mergeability struct { | ||
| 77 | - | cfg MergeabilityConfig | 51 | + | cfg MergeabilityConfig |
| 78 | - | q *mergeQueue | 52 | + | ctx context.Context |
| 79 | - | ctx context.Context | 53 | + | cancel context.CancelFunc |
| 80 | - | cancel context.CancelFunc | 54 | + | wg sync.WaitGroup |
| 81 | - | wg sync.WaitGroup | 55 | + | mu sync.Mutex |
| 82 | - | 56 | + | dirty map[domain.RepoName]bool | |
| 83 | - | retryMu sync.Mutex | 57 | + | order []domain.RepoName |
| 84 | - | retries map[retryKey]int // attempts so far of the scheduled or failing work | 58 | + | running, again map[domain.RepoName]bool |
| 85 | - | scheduled map[retryKey]bool | 59 | + | wake chan struct{} |
| 86 | - | heap []retryItem // sorted by due time | 60 | + | retryMu sync.Mutex |
| 87 | - | retryKick chan struct{} | 61 | + | retries map[domain.RepoName]int |
| 88 | - | closed bool | 62 | + | scheduled map[domain.RepoName]bool |
| 63 | + | heap []retryItem | |||
| 64 | + | retryKick chan struct{} | |||
| 65 | + | closed bool | |||
| 89 | } | 66 | } | ||
| 90 | 67 | ||||
| 91 | - | // MergeabilityHook is what the merge request use cases need from the | 68 | + | // MergeabilityHook is the unchanged trigger and reader contract. |
| 92 | - | // mergeability service: queue checks, read results and drop the record of a | |||
| 93 | - | // request that is no longer open. | |||
| 94 | type MergeabilityHook interface { | 69 | type MergeabilityHook interface { | ||
| 95 | ports.MergeabilityTrigger | 70 | ports.MergeabilityTrigger | ||
| 96 | ports.MergeabilityReader | 71 | ports.MergeabilityReader | ||
| 97 | - | // OnClosed deletes the record of a merge request that left the open state. | |||
| 98 | - | OnClosed(ctx context.Context, mr domain.MergeRequest) | |||
| 99 | } | 72 | } | ||
| 100 | 73 | ||||
| 101 | var _ MergeabilityHook = (*Mergeability)(nil) | 74 | var _ MergeabilityHook = (*Mergeability)(nil) | ||
| 102 | 75 | ||||
| 103 | - | // NewMergeability builds the service. Call Start to run it, Close to stop it. | 76 | + | // NewMergeability builds a reconciler whose lifetime is controlled by Close. |
| 104 | func NewMergeability(ctx context.Context, cfg MergeabilityConfig) *Mergeability { | 77 | func NewMergeability(ctx context.Context, cfg MergeabilityConfig) *Mergeability { | ||
| 105 | if cfg.Workers <= 0 { | 78 | if cfg.Workers <= 0 { | ||
| 106 | cfg.Workers = defaultMergeabilityWorkers | 79 | cfg.Workers = defaultMergeabilityWorkers | ||
| 107 | } | 80 | } | ||
| 108 | - | if cfg.Queue <= 0 { | |||
| 109 | - | cfg.Queue = defaultMergeabilityQueue | |||
| 110 | - | } | |||
| 111 | if cfg.After == nil { | 81 | if cfg.After == nil { | ||
| 112 | cfg.After = time.After | 82 | cfg.After = time.After | ||
| 113 | } | 83 | } | ||
| 114 | - | // The workers outlive the cancellation of ctx: Close controls their life. | 84 | + | ctx, cancel := context.WithCancel(context.WithoutCancel(ctx)) |
| 115 | - | wctx, cancel := context.WithCancel(context.WithoutCancel(ctx)) | 85 | + | return &Mergeability{cfg: cfg, ctx: ctx, cancel: cancel, |
| 116 | - | return &Mergeability{ | 86 | + | dirty: map[domain.RepoName]bool{}, running: map[domain.RepoName]bool{}, again: map[domain.RepoName]bool{}, wake: make(chan struct{}, 1), |
| 117 | - | cfg: cfg, q: newMergeQueue(cfg.Queue), ctx: wctx, cancel: cancel, | 87 | + | retries: map[domain.RepoName]int{}, scheduled: map[domain.RepoName]bool{}, retryKick: make(chan struct{}, 1)} |
| 118 | - | retries: map[retryKey]int{}, scheduled: map[retryKey]bool{}, retryKick: make(chan struct{}, 1), | |||
| 119 | - | } | |||
| 120 | } | 88 | } | ||
| 121 | 89 | ||||
| 122 | - | // Start rescans the open merge requests of every repository (the records may | 90 | + | // Start starts workers and discovers repositories, retrying failed discovery. |
| 123 | - | // be missing or stale after a restart) and starts the workers. | |||
| 124 | func (s *Mergeability) Start() { | 91 | func (s *Mergeability) Start() { | ||
| 125 | - | s.wg.Add(1) | 92 | + | s.wg.Add(2 + s.cfg.Workers) |
| 126 | go s.scheduler() | 93 | go s.scheduler() | ||
| 127 | - | s.rescanAll() | 94 | + | go func() { |
| 95 | + | defer s.wg.Done() | |||
| 96 | + | for attempt := 0; attempt <= mergeabilityRetries; attempt++ { | |||
| 97 | + | repos, err := s.cfg.Meta.List(s.ctx) | |||
| 98 | + | if err == nil { | |||
| 99 | + | for _, repo := range repos { | |||
| 100 | + | s.markDirty(repo.Name) | |||
| 101 | + | } | |||
| 102 | + | return | |||
| 103 | + | } | |||
| 104 | + | if attempt == mergeabilityRetries { | |||
| 105 | + | return | |||
| 106 | + | } | |||
| 107 | + | select { | |||
| 108 | + | case <-s.ctx.Done(): | |||
| 109 | + | return | |||
| 110 | + | case <-s.cfg.After(mergeBackoff(attempt + 1)): | |||
| 111 | + | } | |||
| 112 | + | } | |||
| 113 | + | }() | |||
| 128 | for range s.cfg.Workers { | 114 | for range s.cfg.Workers { | ||
| 129 | - | s.wg.Add(1) | |||
| 130 | go s.loop() | 115 | go s.loop() | ||
| 131 | } | 116 | } | ||
| 132 | } | 117 | } | ||
| 133 | 118 | ||||
| 134 | - | // Close stops the workers and waits for them. | 119 | + | // Close cancels workers and pending retry waits, then waits for them. |
| 135 | func (s *Mergeability) Close(_ context.Context) { | 120 | func (s *Mergeability) Close(_ context.Context) { | ||
| 136 | s.retryMu.Lock() | 121 | s.retryMu.Lock() | ||
| 137 | s.closed = true | 122 | s.closed = true | ||
| @@ -140,16 +125,160 @@ func (s *Mergeability) Close(_ context.Context) { | |||||
| 140 | s.wg.Wait() | 125 | s.wg.Wait() | ||
| 141 | } | 126 | } | ||
| 142 | 127 | ||||
| 128 | + | func (s *Mergeability) signal() { | |||
| 129 | + | select { | |||
| 130 | + | case s.wake <- struct{}{}: | |||
| 131 | + | default: | |||
| 132 | + | } | |||
| 133 | + | } | |||
| 134 | + | ||||
| 135 | + | func (s *Mergeability) markDirty(repo domain.RepoName) { | |||
| 136 | + | s.mu.Lock() | |||
| 137 | + | if s.running[repo] { | |||
| 138 | + | s.again[repo] = true | |||
| 139 | + | } else if !s.dirty[repo] { | |||
| 140 | + | s.dirty[repo] = true | |||
| 141 | + | s.order = append(s.order, repo) | |||
| 142 | + | } | |||
| 143 | + | s.mu.Unlock() | |||
| 144 | + | s.signal() | |||
| 145 | + | } | |||
| 146 | + | ||||
| 147 | + | func (s *Mergeability) next() (domain.RepoName, bool) { | |||
| 148 | + | s.mu.Lock() | |||
| 149 | + | defer s.mu.Unlock() | |||
| 150 | + | if len(s.order) == 0 { | |||
| 151 | + | return domain.RepoName{}, false | |||
| 152 | + | } | |||
| 153 | + | repo := s.order[0] | |||
| 154 | + | s.order = s.order[1:] | |||
| 155 | + | delete(s.dirty, repo) | |||
| 156 | + | s.running[repo] = true | |||
| 157 | + | if len(s.order) > 0 { | |||
| 158 | + | s.signal() | |||
| 159 | + | } | |||
| 160 | + | return repo, true | |||
| 161 | + | } | |||
| 162 | + | ||||
| 163 | + | func (s *Mergeability) loop() { | |||
| 164 | + | defer s.wg.Done() | |||
| 165 | + | for s.ctx.Err() == nil { | |||
| 166 | + | repo, found := s.next() | |||
| 167 | + | if !found { | |||
| 168 | + | select { | |||
| 169 | + | case <-s.ctx.Done(): | |||
| 170 | + | return | |||
| 171 | + | case <-s.wake: | |||
| 172 | + | } | |||
| 173 | + | continue | |||
| 174 | + | } | |||
| 175 | + | if s.reconcile(repo) { | |||
| 176 | + | s.retryDone(repo) | |||
| 177 | + | } else { | |||
| 178 | + | s.retry(repo) | |||
| 179 | + | } | |||
| 180 | + | s.mu.Lock() | |||
| 181 | + | delete(s.running, repo) | |||
| 182 | + | if s.again[repo] { | |||
| 183 | + | delete(s.again, repo) | |||
| 184 | + | s.dirty[repo] = true | |||
| 185 | + | s.order = append(s.order, repo) | |||
| 186 | + | s.signal() | |||
| 187 | + | } | |||
| 188 | + | s.mu.Unlock() | |||
| 189 | + | } | |||
| 190 | + | } | |||
| 191 | + | ||||
| 192 | + | // OnOpened implements ports.MergeabilityTrigger. | |||
| 193 | + | func (s *Mergeability) OnOpened(_ context.Context, mr domain.MergeRequest) { s.markDirty(mr.Repo) } | |||
| 194 | + | ||||
| 195 | + | // OnBranchesMoved dirties the repository for any branch update. | |||
| 196 | + | func (s *Mergeability) OnBranchesMoved(_ context.Context, repo domain.RepoName, updates []domain.RefUpdate) { | |||
| 197 | + | for _, update := range updates { | |||
| 198 | + | if strings.HasPrefix(update.Ref, "refs/heads/") { | |||
| 199 | + | s.markDirty(repo) | |||
| 200 | + | return | |||
| 201 | + | } | |||
| 202 | + | } | |||
| 203 | + | } | |||
| 204 | + | ||||
| 205 | + | // Read returns a current record, or checking and requests reconciliation. | |||
| 206 | + | func (s *Mergeability) Read(ctx context.Context, mr domain.MergeRequest, targetTip domain.ObjectID) domain.Mergeability { | |||
| 207 | + | if !mr.HeadID.IsZero() && !targetTip.IsZero() { | |||
| 208 | + | rec, err := s.cfg.Store.GetMergeability(ctx, mr.Repo, mr.Number) | |||
| 209 | + | if err == nil && rec.IsFor(targetTip, mr.HeadID) { | |||
| 210 | + | return rec | |||
| 211 | + | } | |||
| 212 | + | } | |||
| 213 | + | s.markDirty(mr.Repo) | |||
| 214 | + | return domain.Mergeability{Repo: mr.Repo, Number: mr.Number, State: domain.MergeabilityChecking} | |||
| 215 | + | } | |||
| 216 | + | ||||
| 217 | + | // reconcile settles each open request for the tips read in this pass. A push | |||
| 218 | + | // during computation requests another pass; old-tip records are harmless. | |||
| 219 | + | func (s *Mergeability) reconcile(repo domain.RepoName) bool { | |||
| 220 | + | ctx := s.ctx | |||
| 221 | + | meta, err := s.cfg.Meta.Get(ctx, repo) | |||
| 222 | + | if errors.Is(err, domain.ErrNotFound) { | |||
| 223 | + | return true | |||
| 224 | + | } | |||
| 225 | + | if err != nil { | |||
| 226 | + | return false | |||
| 227 | + | } | |||
| 228 | + | mrs, err := s.cfg.Store.ListOpen(ctx, repo) | |||
| 229 | + | if err != nil { | |||
| 230 | + | return false | |||
| 231 | + | } | |||
| 232 | + | ok := true | |||
| 233 | + | for _, mr := range mrs { | |||
| 234 | + | if ctx.Err() != nil { | |||
| 235 | + | return true | |||
| 236 | + | } | |||
| 237 | + | head, tip, found, transient := s.tips(ctx, mr) | |||
| 238 | + | if !found { | |||
| 239 | + | ok = ok && !transient | |||
| 240 | + | continue | |||
| 241 | + | } | |||
| 242 | + | prev, err := s.cfg.Store.GetMergeability(ctx, repo, mr.Number) | |||
| 243 | + | if err != nil && !errors.Is(err, domain.ErrNotFound) { | |||
| 244 | + | ok = false | |||
| 245 | + | continue | |||
| 246 | + | } | |||
| 247 | + | if prev.IsFor(tip, head) { | |||
| 248 | + | continue | |||
| 249 | + | } | |||
| 250 | + | res, err := s.compute(ctx, repo, mr.Number, tip, head) | |||
| 251 | + | if err != nil { | |||
| 252 | + | ok = false | |||
| 253 | + | continue | |||
| 254 | + | } | |||
| 255 | + | res.Version = prev.Version | |||
| 256 | + | err = s.cfg.Store.PutMergeability(ctx, res) | |||
| 257 | + | if errors.Is(err, domain.ErrNotFound) { | |||
| 258 | + | continue | |||
| 259 | + | } | |||
| 260 | + | if err != nil { | |||
| 261 | + | ok = false | |||
| 262 | + | continue | |||
| 263 | + | } | |||
| 264 | + | s.cfg.Bus.Publish(ctx, domain.MergeRequestMergeability{ | |||
| 265 | + | Repo: repo, Number: mr.Number, Mergeability: res, At: res.CheckedAt, | |||
| 266 | + | Visibility: meta.Visibility, CreatedAt: meta.CreatedAt, | |||
| 267 | + | }) | |||
| 268 | + | } | |||
| 269 | + | return ok | |||
| 270 | + | } | |||
| 271 | + | ||||
| 143 | // retryItem is one scheduled retry. | 272 | // retryItem is one scheduled retry. | ||
| 144 | type retryItem struct { | 273 | type retryItem struct { | ||
| 145 | due time.Time | 274 | due time.Time | ||
| 146 | - | key retryKey | 275 | + | key domain.RepoName |
| 147 | } | 276 | } | ||
| 148 | 277 | ||||
| 149 | // retry schedules a retry of key after a bounded exponential backoff. It is | 278 | // retry schedules a retry of key after a bounded exponential backoff. It is | ||
| 150 | // deduplicated per key, gives up after mergeabilityRetries attempts and stops | 279 | // deduplicated per key, gives up after mergeabilityRetries attempts and stops | ||
| 151 | // on Close. All retries share one scheduler goroutine. | 280 | // on Close. All retries share one scheduler goroutine. | ||
| 152 | - | func (s *Mergeability) retry(key retryKey) { | 281 | + | func (s *Mergeability) retry(key domain.RepoName) { |
| 153 | s.retryMu.Lock() | 282 | s.retryMu.Lock() | ||
| 154 | defer s.retryMu.Unlock() | 283 | defer s.retryMu.Unlock() | ||
| 155 | if s.closed || s.scheduled[key] { | 284 | if s.closed || s.scheduled[key] { | ||
| @@ -220,17 +349,17 @@ func (s *Mergeability) scheduler() { | |||||
| 220 | if s.ctx.Err() != nil { | 349 | if s.ctx.Err() != nil { | ||
| 221 | return | 350 | return | ||
| 222 | } | 351 | } | ||
| 223 | - | s.fire(k) | 352 | + | s.markDirty(k) |
| 224 | } | 353 | } | ||
| 225 | } | 354 | } | ||
| 226 | } | 355 | } | ||
| 227 | 356 | ||||
| 228 | // popDue removes the waited item and the ones due as well. | 357 | // popDue removes the waited item and the ones due as well. | ||
| 229 | - | func (s *Mergeability) popDue(waited retryKey) []retryKey { | 358 | + | func (s *Mergeability) popDue(waited domain.RepoName) []domain.RepoName { |
| 230 | s.retryMu.Lock() | 359 | s.retryMu.Lock() | ||
| 231 | defer s.retryMu.Unlock() | 360 | defer s.retryMu.Unlock() | ||
| 232 | now := s.cfg.Clock.Now() | 361 | now := s.cfg.Clock.Now() | ||
| 233 | - | var out []retryKey | 362 | + | var out []domain.RepoName |
| 234 | s.heap = slices.DeleteFunc(s.heap, func(it retryItem) bool { | 363 | s.heap = slices.DeleteFunc(s.heap, func(it retryItem) bool { | ||
| 235 | if it.key != waited && it.due.After(now) { | 364 | if it.key != waited && it.due.After(now) { | ||
| 236 | return false | 365 | return false | ||
| @@ -242,260 +371,17 @@ func (s *Mergeability) popDue(waited retryKey) []retryKey { | |||||
| 242 | return out | 371 | return out | ||
| 243 | } | 372 | } | ||
| 244 | 373 | ||||
| 245 | - | // fire puts the work of a due retry back in the normal queue, so that the | |||
| 246 | - | // worker limits apply. | |||
| 247 | - | func (s *Mergeability) fire(k retryKey) { | |||
| 248 | - | if k.all { | |||
| 249 | - | s.rescanAll() | |||
| 250 | - | return | |||
| 251 | - | } | |||
| 252 | - | s.q.markDirty(k.repo) | |||
| 253 | - | } | |||
| 254 | - | ||||
| 255 | // retryDone forgets the failures of key after a success. | 374 | // retryDone forgets the failures of key after a success. | ||
| 256 | - | func (s *Mergeability) retryDone(key retryKey) { | 375 | + | func (s *Mergeability) retryDone(key domain.RepoName) { |
| 257 | s.retryMu.Lock() | 376 | s.retryMu.Lock() | ||
| 258 | delete(s.retries, key) | 377 | delete(s.retries, key) | ||
| 259 | - | s.retryMu.Unlock() | 378 | + | delete(s.scheduled, key) |
| 260 | - | } | 379 | + | s.heap = slices.DeleteFunc(s.heap, func(it retryItem) bool { return it.key == key }) |
| 261 | - | 380 | + | select { | |
| 262 | - | func (s *Mergeability) rescanAll() { | 381 | + | case s.retryKick <- struct{}{}: |
| 263 | - | repos, err := s.cfg.Meta.List(s.ctx) | 382 | + | default: |
| 264 | - | if err != nil { | |||
| 265 | - | ctxLog(s.ctx).Error().Err(err).Msg("mergeability: list the repositories to rescan") | |||
| 266 | - | s.retry(globalKey) | |||
| 267 | - | return | |||
| 268 | - | } | |||
| 269 | - | s.retryDone(globalKey) | |||
| 270 | - | for _, r := range repos { | |||
| 271 | - | n, err := s.cfg.Store.CountOpen(s.ctx, r.Name) | |||
| 272 | - | if err != nil { | |||
| 273 | - | ctxLog(s.ctx).Error().Err(err).Str("repo", r.Name.String()).Msg("mergeability: count the open merge requests") | |||
| 274 | - | s.q.markDirty(r.Name) | |||
| 275 | - | continue | |||
| 276 | - | } | |||
| 277 | - | if n > 0 { | |||
| 278 | - | s.q.markDirty(r.Name) | |||
| 279 | - | } | |||
| 280 | } | 383 | } | ||
| 281 | - | } | 384 | + | s.retryMu.Unlock() |
| 282 | - | ||||
| 283 | - | // OnOpened implements ports.MergeabilityTrigger. | |||
| 284 | - | func (s *Mergeability) OnOpened(_ context.Context, mr domain.MergeRequest) { | |||
| 285 | - | s.q.add(mergeKey{mr.Repo, mr.Number}) | |||
| 286 | - | } | |||
| 287 | - | ||||
| 288 | - | // OnClosed deletes the record of a merge request that is no longer open. The | |||
| 289 | - | // store already drops it when the request leaves the open state; this makes it | |||
| 290 | - | // explicit and is idempotent. | |||
| 291 | - | func (s *Mergeability) OnClosed(ctx context.Context, mr domain.MergeRequest) { | |||
| 292 | - | if err := s.cfg.Store.DeleteMergeability(context.WithoutCancel(ctx), mr.Repo, mr.Number); err != nil { | |||
| 293 | - | ctxLog(ctx).Error().Err(err).Str("repo", mr.Repo.String()).Int64("number", mr.Number).Msg("mergeability: delete the record of a closed merge request") | |||
| 294 | - | } | |||
| 295 | - | s.q.takeSeen(mergeKey{mr.Repo, mr.Number}) | |||
| 296 | - | } | |||
| 297 | - | ||||
| 298 | - | // OnBranchesMoved implements ports.MergeabilityTrigger: the open merge | |||
| 299 | - | // requests whose source or target is one of the branches are queued. Past the | |||
| 300 | - | // bound, or when the lookup fails, the repository is rescanned. | |||
| 301 | - | func (s *Mergeability) OnBranchesMoved(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate) { | |||
| 302 | - | var branches []string | |||
| 303 | - | for _, u := range updates { | |||
| 304 | - | if b, ok := strings.CutPrefix(u.Ref, "refs/heads/"); ok && !slices.Contains(branches, b) { | |||
| 305 | - | branches = append(branches, b) | |||
| 306 | - | } | |||
| 307 | - | } | |||
| 308 | - | if len(branches) == 0 { | |||
| 309 | - | return | |||
| 310 | - | } | |||
| 311 | - | lookups := []func(context.Context, domain.RepoName, []string, int) ([]int64, bool, error){ | |||
| 312 | - | s.cfg.Store.OpenBySource, s.cfg.Store.OpenByTarget, | |||
| 313 | - | } | |||
| 314 | - | for _, lookup := range lookups { | |||
| 315 | - | numbers, truncated, err := lookup(ctx, repo, branches, s.cfg.Queue) | |||
| 316 | - | if err != nil || truncated { | |||
| 317 | - | if err != nil { | |||
| 318 | - | ctxLog(ctx).Error().Err(err).Str("repo", repo.String()).Msg("mergeability: look up the merge requests of moved branches: the repository is rescanned") | |||
| 319 | - | } | |||
| 320 | - | s.q.markDirty(repo) | |||
| 321 | - | } | |||
| 322 | - | for _, n := range numbers { | |||
| 323 | - | s.q.add(mergeKey{repo, n}) | |||
| 324 | - | } | |||
| 325 | - | } | |||
| 326 | - | } | |||
| 327 | - | ||||
| 328 | - | // Read implements ports.MergeabilityReader. | |||
| 329 | - | func (s *Mergeability) Read(ctx context.Context, mr domain.MergeRequest, targetTip domain.ObjectID) domain.Mergeability { | |||
| 330 | - | checking := domain.Mergeability{Repo: mr.Repo, Number: mr.Number, State: domain.MergeabilityChecking} | |||
| 331 | - | if mr.HeadID.IsZero() || targetTip.IsZero() { | |||
| 332 | - | return checking // a branch is gone: there is nothing to check | |||
| 333 | - | } | |||
| 334 | - | rec, err := s.cfg.Store.GetMergeability(ctx, mr.Repo, mr.Number) | |||
| 335 | - | if err == nil && rec.IsFor(targetTip, mr.HeadID) { | |||
| 336 | - | return rec | |||
| 337 | - | } | |||
| 338 | - | k := mergeKey{mr.Repo, mr.Number} | |||
| 339 | - | s.q.markSeen(k) | |||
| 340 | - | s.q.add(k) | |||
| 341 | - | return checking | |||
| 342 | - | } | |||
| 343 | - | ||||
| 344 | - | func (s *Mergeability) loop() { | |||
| 345 | - | defer s.wg.Done() | |||
| 346 | - | for { | |||
| 347 | - | key, dirty := s.q.next() | |||
| 348 | - | switch { | |||
| 349 | - | case key != nil: | |||
| 350 | - | if !s.runKey(*key) { | |||
| 351 | - | s.retry(repoKey(key.repo)) | |||
| 352 | - | } | |||
| 353 | - | case dirty != nil: | |||
| 354 | - | s.rescan(*dirty) | |||
| 355 | - | default: | |||
| 356 | - | select { | |||
| 357 | - | case <-s.ctx.Done(): | |||
| 358 | - | return | |||
| 359 | - | case <-s.q.wake: | |||
| 360 | - | } | |||
| 361 | - | } | |||
| 362 | - | if s.ctx.Err() != nil { | |||
| 363 | - | return | |||
| 364 | - | } | |||
| 365 | - | } | |||
| 366 | - | } | |||
| 367 | - | ||||
| 368 | - | // runKey checks k, which is marked running, as often as it was queued again | |||
| 369 | - | // meanwhile. It reports false when a check failed transiently; the caller | |||
| 370 | - | // then retries the repository. | |||
| 371 | - | func (s *Mergeability) runKey(k mergeKey) bool { | |||
| 372 | - | ok := true | |||
| 373 | - | for { | |||
| 374 | - | ok = s.check(k) && ok | |||
| 375 | - | if !s.q.finish(k) || s.ctx.Err() != nil { | |||
| 376 | - | return ok | |||
| 377 | - | } | |||
| 378 | - | } | |||
| 379 | - | } | |||
| 380 | - | ||||
| 381 | - | // rescan checks every open merge request of a repository, here and not through | |||
| 382 | - | // the bounded queue, so that a rescan never marks its repository dirty again. | |||
| 383 | - | // It succeeds, and resets the retry budget of the repository, only if the | |||
| 384 | - | // listing and every check succeeded; otherwise the repository is retried once. | |||
| 385 | - | func (s *Mergeability) rescan(repo domain.RepoName) { | |||
| 386 | - | mrs, err := s.cfg.Store.ListOpen(s.ctx, repo) | |||
| 387 | - | if err != nil { | |||
| 388 | - | ctxLog(s.ctx).Error().Err(err).Str("repo", repo.String()).Msg("mergeability: list the open merge requests to rescan") | |||
| 389 | - | s.retry(repoKey(repo)) | |||
| 390 | - | return | |||
| 391 | - | } | |||
| 392 | - | ok := true | |||
| 393 | - | for _, mr := range mrs { | |||
| 394 | - | if s.ctx.Err() != nil { | |||
| 395 | - | return | |||
| 396 | - | } | |||
| 397 | - | k := mergeKey{repo, mr.Number} | |||
| 398 | - | if s.q.tryRun(k) { | |||
| 399 | - | ok = s.runKey(k) && ok | |||
| 400 | - | } | |||
| 401 | - | } | |||
| 402 | - | if ok { | |||
| 403 | - | s.retryDone(repoKey(repo)) | |||
| 404 | - | } else { | |||
| 405 | - | s.retry(repoKey(repo)) | |||
| 406 | - | } | |||
| 407 | - | } | |||
| 408 | - | ||||
| 409 | - | // Check computes the mergeability of a merge request now, on the calling | |||
| 410 | - | // goroutine, as a worker does. It is for tests; the server never calls it. | |||
| 411 | - | func (s *Mergeability) Check(repo domain.RepoName, number int64) { | |||
| 412 | - | k := mergeKey{repo, number} | |||
| 413 | - | if s.q.tryRun(k) && !s.runKey(k) { | |||
| 414 | - | s.retry(repoKey(k.repo)) | |||
| 415 | - | } | |||
| 416 | - | } | |||
| 417 | - | ||||
| 418 | - | // check brings the record of k up to date. It reports false when a transient | |||
| 419 | - | // error (store, Git) dropped the work, which is then retried. | |||
| 420 | - | func (s *Mergeability) check(k mergeKey) (done bool) { | |||
| 421 | - | ctx := s.ctx | |||
| 422 | - | log := ctxLog(ctx).With().Str("repo", k.repo.String()).Int64("number", k.number).Logger() | |||
| 423 | - | for range mergeabilityAttempts { | |||
| 424 | - | mr, err := s.cfg.Store.Get(ctx, k.repo, k.number) | |||
| 425 | - | if err != nil { | |||
| 426 | - | if errors.Is(err, domain.ErrNotFound) { | |||
| 427 | - | s.q.takeSeen(k) | |||
| 428 | - | return true | |||
| 429 | - | } | |||
| 430 | - | if ctx.Err() != nil { | |||
| 431 | - | return true | |||
| 432 | - | } | |||
| 433 | - | log.Error().Err(err).Msg("mergeability: read the merge request") | |||
| 434 | - | return false | |||
| 435 | - | } | |||
| 436 | - | if !mr.IsOpen() { | |||
| 437 | - | s.q.takeSeen(k) | |||
| 438 | - | return true // the store dropped the record when it left the open state | |||
| 439 | - | } | |||
| 440 | - | head, tip, ok, transient := s.tips(ctx, mr) | |||
| 441 | - | if !ok { | |||
| 442 | - | return !transient | |||
| 443 | - | } | |||
| 444 | - | prev, err := s.cfg.Store.GetMergeability(ctx, k.repo, k.number) | |||
| 445 | - | if err != nil && !errors.Is(err, domain.ErrNotFound) { | |||
| 446 | - | if ctx.Err() != nil { | |||
| 447 | - | return true | |||
| 448 | - | } | |||
| 449 | - | log.Error().Err(err).Msg("mergeability: read the record") | |||
| 450 | - | return false | |||
| 451 | - | } | |||
| 452 | - | if prev.IsFor(tip, head) { | |||
| 453 | - | if s.q.takeSeen(k) && s.publish(ctx, prev) != nil { // a reader saw "checking" before this record | |||
| 454 | - | s.q.markSeen(k) | |||
| 455 | - | return false | |||
| 456 | - | } | |||
| 457 | - | return true | |||
| 458 | - | } | |||
| 459 | - | res, err := s.compute(ctx, k, tip, head) | |||
| 460 | - | if err != nil { | |||
| 461 | - | if ctx.Err() != nil { | |||
| 462 | - | return true | |||
| 463 | - | } | |||
| 464 | - | log.Error().Err(err).Msg("mergeability: compute") | |||
| 465 | - | return false | |||
| 466 | - | } | |||
| 467 | - | // The branches may have moved during the computation: the result is | |||
| 468 | - | // then for tips nobody asks about, and is dropped. | |||
| 469 | - | if h2, t2, ok, transient := s.tips(ctx, mr); !ok { | |||
| 470 | - | return !transient | |||
| 471 | - | } else if h2 != head || t2 != tip { | |||
| 472 | - | continue | |||
| 473 | - | } | |||
| 474 | - | res.Version = prev.Version | |||
| 475 | - | switch err := s.cfg.Store.PutMergeability(ctx, res); { | |||
| 476 | - | case errors.Is(err, domain.ErrConflict): | |||
| 477 | - | continue | |||
| 478 | - | case errors.Is(err, domain.ErrNotFound): | |||
| 479 | - | s.q.takeSeen(k) | |||
| 480 | - | return true // closed meanwhile | |||
| 481 | - | case err != nil: | |||
| 482 | - | if ctx.Err() != nil { | |||
| 483 | - | return true | |||
| 484 | - | } | |||
| 485 | - | log.Error().Err(err).Msg("mergeability: store the record") | |||
| 486 | - | return false | |||
| 487 | - | } | |||
| 488 | - | seen := s.q.takeSeen(k) | |||
| 489 | - | if seen || prev.State == "" || changedMergeability(prev, res) { | |||
| 490 | - | if s.publish(ctx, res) != nil { | |||
| 491 | - | s.q.markSeen(k) // the next check publishes it | |||
| 492 | - | return false | |||
| 493 | - | } | |||
| 494 | - | } | |||
| 495 | - | return true | |||
| 496 | - | } | |||
| 497 | - | s.q.add(k) // kept losing races: try again later | |||
| 498 | - | return true | |||
| 499 | } | 385 | } | ||
| 500 | 386 | ||||
| 501 | // tips reads the heads of the source and the target of mr. ok is false when | 387 | // tips reads the heads of the source and the target of mr. ok is false when | ||
| @@ -512,7 +398,6 @@ func (s *Mergeability) tips(ctx context.Context, mr domain.MergeRequest) (head, | |||||
| 512 | } | 398 | } | ||
| 513 | transient = ctx.Err() == nil | 399 | transient = ctx.Err() == nil | ||
| 514 | } | 400 | } | ||
| 515 | - | s.q.takeSeen(mergeKey{mr.Repo, mr.Number}) | |||
| 516 | return domain.ObjectID{}, domain.ObjectID{}, false, transient | 401 | return domain.ObjectID{}, domain.ObjectID{}, false, transient | ||
| 517 | case herr != nil || terr != nil: | 402 | case herr != nil || terr != nil: | ||
| 518 | if ctx.Err() != nil { | 403 | if ctx.Err() != nil { | ||
| @@ -525,12 +410,12 @@ func (s *Mergeability) tips(ctx context.Context, mr domain.MergeRequest) (head, | |||||
| 525 | } | 410 | } | ||
| 526 | 411 | ||||
| 527 | // compute runs the Git reads that decide the state of head against tip. | 412 | // compute runs the Git reads that decide the state of head against tip. | ||
| 528 | - | func (s *Mergeability) compute(ctx context.Context, k mergeKey, tip, head domain.ObjectID) (domain.Mergeability, error) { | 413 | + | func (s *Mergeability) compute(ctx context.Context, repo domain.RepoName, number int64, tip, head domain.ObjectID) (domain.Mergeability, error) { |
| 529 | - | m := domain.Mergeability{Repo: k.repo, Number: k.number, TargetID: tip, HeadID: head, CheckedAt: s.cfg.Clock.Now().UTC()} | 414 | + | m := domain.Mergeability{Repo: repo, Number: number, TargetID: tip, HeadID: head, CheckedAt: s.cfg.Clock.Now().UTC()} |
| 530 | ancestor := tip == head | 415 | ancestor := tip == head | ||
| 531 | if !ancestor { | 416 | if !ancestor { | ||
| 532 | var err error | 417 | var err error | ||
| 533 | - | if ancestor, err = s.cfg.Git.IsAncestor(ctx, k.repo, tip, head); err != nil { | 418 | + | if ancestor, err = s.cfg.Git.IsAncestor(ctx, repo, tip, head); err != nil { |
| 534 | return m, err | 419 | return m, err | ||
| 535 | } | 420 | } | ||
| 536 | } | 421 | } | ||
| @@ -538,7 +423,7 @@ func (s *Mergeability) compute(ctx context.Context, k mergeKey, tip, head domain | |||||
| 538 | m.State = domain.MergeabilityUpToDate | 423 | m.State = domain.MergeabilityUpToDate | ||
| 539 | return m, nil | 424 | return m, nil | ||
| 540 | } | 425 | } | ||
| 541 | - | tree, err := s.cfg.Git.MergeTree(ctx, k.repo, tip, head) | 426 | + | tree, err := s.cfg.Git.MergeTree(ctx, repo, tip, head) |
| 542 | if err != nil { | 427 | if err != nil { | ||
| 543 | return m, err | 428 | return m, err | ||
| 544 | } | 429 | } | ||
| @@ -549,34 +434,10 @@ func (s *Mergeability) compute(ctx context.Context, k mergeKey, tip, head domain | |||||
| 549 | return m, nil | 434 | return m, nil | ||
| 550 | } | 435 | } | ||
| 551 | m.State = domain.MergeabilityBehind | 436 | m.State = domain.MergeabilityBehind | ||
| 552 | - | n, err := s.cfg.Git.CountBehind(ctx, k.repo, head, tip, domain.BehindLimit) | 437 | + | n, err := s.cfg.Git.CountBehind(ctx, repo, head, tip, domain.BehindLimit) |
| 553 | if err != nil { | 438 | if err != nil { | ||
| 554 | return m, err | 439 | return m, err | ||
| 555 | } | 440 | } | ||
| 556 | m.Behind, m.BehindCapped = domain.BoundBehind(n) | 441 | m.Behind, m.BehindCapped = domain.BoundBehind(n) | ||
| 557 | return m, nil | 442 | return m, nil | ||
| 558 | } | 443 | } | ||
| 559 | - | ||||
| 560 | - | // changedMergeability reports whether what a reader shows differs. | |||
| 561 | - | func changedMergeability(a, b domain.Mergeability) bool { | |||
| 562 | - | return a.State != b.State || a.Behind != b.Behind || a.BehindCapped != b.BehindCapped || | |||
| 563 | - | a.ConflictPathsTruncated != b.ConflictPathsTruncated || !slices.Equal(a.ConflictPaths, b.ConflictPaths) | |||
| 564 | - | } | |||
| 565 | - | ||||
| 566 | - | // publish sends the result on the bus. It fails on a transient error; a | |||
| 567 | - | // repository that is gone is terminal and not an error. | |||
| 568 | - | func (s *Mergeability) publish(ctx context.Context, m domain.Mergeability) error { | |||
| 569 | - | repo, err := s.cfg.Meta.Get(ctx, m.Repo) | |||
| 570 | - | if err != nil { | |||
| 571 | - | if errors.Is(err, domain.ErrNotFound) || ctx.Err() != nil { | |||
| 572 | - | return nil | |||
| 573 | - | } | |||
| 574 | - | ctxLog(ctx).Error().Err(err).Str("repo", m.Repo.String()).Msg("mergeability: read the repository to publish a result") | |||
| 575 | - | return err | |||
| 576 | - | } | |||
| 577 | - | s.cfg.Bus.Publish(ctx, domain.MergeRequestMergeability{ | |||
| 578 | - | Repo: m.Repo, Number: m.Number, Mergeability: m, At: m.CheckedAt, | |||
| 579 | - | Visibility: repo.Visibility, CreatedAt: repo.CreatedAt, | |||
| 580 | - | }) | |||
| 581 | - | return nil | |||
| 582 | - | } | |||
D internal/usecase/mergeability_queue.go +0 -170
| @@ -1,170 +0,0 @@ | |||
| 1 | - | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | - | ||
| 3 | - | package usecase | |
| 4 | - | ||
| 5 | - | import ( | |
| 6 | - | "sync" | |
| 7 | - | ||
| 8 | - | "bnema.dev/ermite/internal/domain" | |
| 9 | - | ) | |
| 10 | - | ||
| 11 | - | // mergeKey names one open merge request. | |
| 12 | - | type mergeKey struct { | |
| 13 | - | repo domain.RepoName | |
| 14 | - | number int64 | |
| 15 | - | } | |
| 16 | - | ||
| 17 | - | // mergeQueue is the bounded work queue of the mergeability checks: a key | |
| 18 | - | // waits at most once (pending), is checked by one worker at a time (running), | |
| 19 | - | // and a key queued while it runs is checked again afterwards (again). When | |
| 20 | - | // the queue is full the repository is marked dirty instead: a worker then | |
| 21 | - | // rescans all its open merge requests. | |
| 22 | - | type mergeQueue struct { | |
| 23 | - | mu sync.Mutex | |
| 24 | - | max int | |
| 25 | - | pending map[mergeKey]bool | |
| 26 | - | order []mergeKey | |
| 27 | - | running map[mergeKey]bool | |
| 28 | - | again map[mergeKey]bool | |
| 29 | - | dirty map[domain.RepoName]bool | |
| 30 | - | dirtyOrder []domain.RepoName | |
| 31 | - | // seen holds the keys a reader answered "checking" for: the result of | |
| 32 | - | // their next check is published whatever it is. | |
| 33 | - | seen map[mergeKey]bool | |
| 34 | - | wake chan struct{} | |
| 35 | - | // preferDirty alternates the service between pending keys and dirty | |
| 36 | - | // repositories, so that neither starves the other. | |
| 37 | - | preferDirty bool | |
| 38 | - | } | |
| 39 | - | ||
| 40 | - | func newMergeQueue(max int) *mergeQueue { | |
| 41 | - | return &mergeQueue{ | |
| 42 | - | max: max, pending: map[mergeKey]bool{}, running: map[mergeKey]bool{}, again: map[mergeKey]bool{}, | |
| 43 | - | dirty: map[domain.RepoName]bool{}, seen: map[mergeKey]bool{}, wake: make(chan struct{}, 1), | |
| 44 | - | } | |
| 45 | - | } | |
| 46 | - | ||
| 47 | - | func (q *mergeQueue) signal() { | |
| 48 | - | select { | |
| 49 | - | case q.wake <- struct{}{}: | |
| 50 | - | default: | |
| 51 | - | } | |
| 52 | - | } | |
| 53 | - | ||
| 54 | - | // add queues a check. The caller does not hold mu. | |
| 55 | - | func (q *mergeQueue) add(k mergeKey) { | |
| 56 | - | q.mu.Lock() | |
| 57 | - | switch { | |
| 58 | - | case q.running[k]: | |
| 59 | - | q.again[k] = true | |
| 60 | - | case q.pending[k]: | |
| 61 | - | case len(q.pending) < q.max: | |
| 62 | - | q.pending[k] = true | |
| 63 | - | q.order = append(q.order, k) | |
| 64 | - | default: | |
| 65 | - | q.markDirtyLocked(k.repo) | |
| 66 | - | } | |
| 67 | - | q.mu.Unlock() | |
| 68 | - | q.signal() | |
| 69 | - | } | |
| 70 | - | ||
| 71 | - | func (q *mergeQueue) markDirty(repo domain.RepoName) { | |
| 72 | - | q.mu.Lock() | |
| 73 | - | q.markDirtyLocked(repo) | |
| 74 | - | q.mu.Unlock() | |
| 75 | - | q.signal() | |
| 76 | - | } | |
| 77 | - | ||
| 78 | - | func (q *mergeQueue) markDirtyLocked(repo domain.RepoName) { | |
| 79 | - | if !q.dirty[repo] { | |
| 80 | - | q.dirty[repo] = true | |
| 81 | - | q.dirtyOrder = append(q.dirtyOrder, repo) | |
| 82 | - | } | |
| 83 | - | } | |
| 84 | - | ||
| 85 | - | // next takes the next check, which it marks running, or the next dirty | |
| 86 | - | // repository to rescan, alternating between the two when both wait. | |
| 87 | - | func (q *mergeQueue) next() (key *mergeKey, dirty *domain.RepoName) { | |
| 88 | - | q.mu.Lock() | |
| 89 | - | defer q.mu.Unlock() | |
| 90 | - | if q.preferDirty && len(q.dirtyOrder) > 0 { | |
| 91 | - | q.preferDirty = false | |
| 92 | - | r := q.dirtyOrder[0] | |
| 93 | - | q.dirtyOrder = q.dirtyOrder[1:] | |
| 94 | - | delete(q.dirty, r) | |
| 95 | - | dirty = &r | |
| 96 | - | q.signalIfWaitingLocked() | |
| 97 | - | return nil, dirty | |
| 98 | - | } | |
| 99 | - | for key == nil && len(q.order) > 0 { | |
| 100 | - | k := q.order[0] | |
| 101 | - | q.order = q.order[1:] | |
| 102 | - | if !q.pending[k] { | |
| 103 | - | continue // claimed meanwhile by tryRun or an earlier entry | |
| 104 | - | } | |
| 105 | - | delete(q.pending, k) | |
| 106 | - | if q.running[k] { | |
| 107 | - | q.again[k] = true // never run a key concurrently | |
| 108 | - | continue | |
| 109 | - | } | |
| 110 | - | q.running[k] = true | |
| 111 | - | key = &k | |
| 112 | - | } | |
| 113 | - | q.preferDirty = key != nil | |
| 114 | - | if key == nil && len(q.dirtyOrder) > 0 { | |
| 115 | - | r := q.dirtyOrder[0] | |
| 116 | - | q.dirtyOrder = q.dirtyOrder[1:] | |
| 117 | - | delete(q.dirty, r) | |
| 118 | - | dirty = &r | |
| 119 | - | } | |
| 120 | - | q.signalIfWaitingLocked() | |
| 121 | - | return key, dirty | |
| 122 | - | } | |
| 123 | - | ||
| 124 | - | func (q *mergeQueue) signalIfWaitingLocked() { | |
| 125 | - | if len(q.order) > 0 || len(q.dirtyOrder) > 0 { | |
| 126 | - | q.signal() // let another worker take the rest | |
| 127 | - | } | |
| 128 | - | } | |
| 129 | - | ||
| 130 | - | // tryRun marks k running for a rescan; when it already runs it asks for | |
| 131 | - | // another pass instead and reports false. | |
| 132 | - | func (q *mergeQueue) tryRun(k mergeKey) bool { | |
| 133 | - | q.mu.Lock() | |
| 134 | - | defer q.mu.Unlock() | |
| 135 | - | if q.running[k] { | |
| 136 | - | q.again[k] = true | |
| 137 | - | return false | |
| 138 | - | } | |
| 139 | - | delete(q.pending, k) // a pending check is covered by this run | |
| 140 | - | q.running[k] = true | |
| 141 | - | return true | |
| 142 | - | } | |
| 143 | - | ||
| 144 | - | // finish ends a pass of a running key; it reports whether another is due. | |
| 145 | - | func (q *mergeQueue) finish(k mergeKey) (again bool) { | |
| 146 | - | q.mu.Lock() | |
| 147 | - | defer q.mu.Unlock() | |
| 148 | - | if q.again[k] { | |
| 149 | - | delete(q.again, k) | |
| 150 | - | return true | |
| 151 | - | } | |
| 152 | - | delete(q.running, k) | |
| 153 | - | return false | |
| 154 | - | } | |
| 155 | - | ||
| 156 | - | // markSeen records that a reader was told "checking" for k. | |
| 157 | - | func (q *mergeQueue) markSeen(k mergeKey) { | |
| 158 | - | q.mu.Lock() | |
| 159 | - | q.seen[k] = true | |
| 160 | - | q.mu.Unlock() | |
| 161 | - | } | |
| 162 | - | ||
| 163 | - | // takeSeen reports and clears the seen flag. | |
| 164 | - | func (q *mergeQueue) takeSeen(k mergeKey) bool { | |
| 165 | - | q.mu.Lock() | |
| 166 | - | defer q.mu.Unlock() | |
| 167 | - | s := q.seen[k] | |
| 168 | - | delete(q.seen, k) | |
| 169 | - | return s | |
| 170 | - | } | |
| @@ -1,170 +0,0 @@ | |||||
| 1 | - | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | - | ||||
| 3 | - | package usecase | |||
| 4 | - | ||||
| 5 | - | import ( | |||
| 6 | - | "sync" | |||
| 7 | - | ||||
| 8 | - | "bnema.dev/ermite/internal/domain" | |||
| 9 | - | ) | |||
| 10 | - | ||||
| 11 | - | // mergeKey names one open merge request. | |||
| 12 | - | type mergeKey struct { | |||
| 13 | - | repo domain.RepoName | |||
| 14 | - | number int64 | |||
| 15 | - | } | |||
| 16 | - | ||||
| 17 | - | // mergeQueue is the bounded work queue of the mergeability checks: a key | |||
| 18 | - | // waits at most once (pending), is checked by one worker at a time (running), | |||
| 19 | - | // and a key queued while it runs is checked again afterwards (again). When | |||
| 20 | - | // the queue is full the repository is marked dirty instead: a worker then | |||
| 21 | - | // rescans all its open merge requests. | |||
| 22 | - | type mergeQueue struct { | |||
| 23 | - | mu sync.Mutex | |||
| 24 | - | max int | |||
| 25 | - | pending map[mergeKey]bool | |||
| 26 | - | order []mergeKey | |||
| 27 | - | running map[mergeKey]bool | |||
| 28 | - | again map[mergeKey]bool | |||
| 29 | - | dirty map[domain.RepoName]bool | |||
| 30 | - | dirtyOrder []domain.RepoName | |||
| 31 | - | // seen holds the keys a reader answered "checking" for: the result of | |||
| 32 | - | // their next check is published whatever it is. | |||
| 33 | - | seen map[mergeKey]bool | |||
| 34 | - | wake chan struct{} | |||
| 35 | - | // preferDirty alternates the service between pending keys and dirty | |||
| 36 | - | // repositories, so that neither starves the other. | |||
| 37 | - | preferDirty bool | |||
| 38 | - | } | |||
| 39 | - | ||||
| 40 | - | func newMergeQueue(max int) *mergeQueue { | |||
| 41 | - | return &mergeQueue{ | |||
| 42 | - | max: max, pending: map[mergeKey]bool{}, running: map[mergeKey]bool{}, again: map[mergeKey]bool{}, | |||
| 43 | - | dirty: map[domain.RepoName]bool{}, seen: map[mergeKey]bool{}, wake: make(chan struct{}, 1), | |||
| 44 | - | } | |||
| 45 | - | } | |||
| 46 | - | ||||
| 47 | - | func (q *mergeQueue) signal() { | |||
| 48 | - | select { | |||
| 49 | - | case q.wake <- struct{}{}: | |||
| 50 | - | default: | |||
| 51 | - | } | |||
| 52 | - | } | |||
| 53 | - | ||||
| 54 | - | // add queues a check. The caller does not hold mu. | |||
| 55 | - | func (q *mergeQueue) add(k mergeKey) { | |||
| 56 | - | q.mu.Lock() | |||
| 57 | - | switch { | |||
| 58 | - | case q.running[k]: | |||
| 59 | - | q.again[k] = true | |||
| 60 | - | case q.pending[k]: | |||
| 61 | - | case len(q.pending) < q.max: | |||
| 62 | - | q.pending[k] = true | |||
| 63 | - | q.order = append(q.order, k) | |||
| 64 | - | default: | |||
| 65 | - | q.markDirtyLocked(k.repo) | |||
| 66 | - | } | |||
| 67 | - | q.mu.Unlock() | |||
| 68 | - | q.signal() | |||
| 69 | - | } | |||
| 70 | - | ||||
| 71 | - | func (q *mergeQueue) markDirty(repo domain.RepoName) { | |||
| 72 | - | q.mu.Lock() | |||
| 73 | - | q.markDirtyLocked(repo) | |||
| 74 | - | q.mu.Unlock() | |||
| 75 | - | q.signal() | |||
| 76 | - | } | |||
| 77 | - | ||||
| 78 | - | func (q *mergeQueue) markDirtyLocked(repo domain.RepoName) { | |||
| 79 | - | if !q.dirty[repo] { | |||
| 80 | - | q.dirty[repo] = true | |||
| 81 | - | q.dirtyOrder = append(q.dirtyOrder, repo) | |||
| 82 | - | } | |||
| 83 | - | } | |||
| 84 | - | ||||
| 85 | - | // next takes the next check, which it marks running, or the next dirty | |||
| 86 | - | // repository to rescan, alternating between the two when both wait. | |||
| 87 | - | func (q *mergeQueue) next() (key *mergeKey, dirty *domain.RepoName) { | |||
| 88 | - | q.mu.Lock() | |||
| 89 | - | defer q.mu.Unlock() | |||
| 90 | - | if q.preferDirty && len(q.dirtyOrder) > 0 { | |||
| 91 | - | q.preferDirty = false | |||
| 92 | - | r := q.dirtyOrder[0] | |||
| 93 | - | q.dirtyOrder = q.dirtyOrder[1:] | |||
| 94 | - | delete(q.dirty, r) | |||
| 95 | - | dirty = &r | |||
| 96 | - | q.signalIfWaitingLocked() | |||
| 97 | - | return nil, dirty | |||
| 98 | - | } | |||
| 99 | - | for key == nil && len(q.order) > 0 { | |||
| 100 | - | k := q.order[0] | |||
| 101 | - | q.order = q.order[1:] | |||
| 102 | - | if !q.pending[k] { | |||
| 103 | - | continue // claimed meanwhile by tryRun or an earlier entry | |||
| 104 | - | } | |||
| 105 | - | delete(q.pending, k) | |||
| 106 | - | if q.running[k] { | |||
| 107 | - | q.again[k] = true // never run a key concurrently | |||
| 108 | - | continue | |||
| 109 | - | } | |||
| 110 | - | q.running[k] = true | |||
| 111 | - | key = &k | |||
| 112 | - | } | |||
| 113 | - | q.preferDirty = key != nil | |||
| 114 | - | if key == nil && len(q.dirtyOrder) > 0 { | |||
| 115 | - | r := q.dirtyOrder[0] | |||
| 116 | - | q.dirtyOrder = q.dirtyOrder[1:] | |||
| 117 | - | delete(q.dirty, r) | |||
| 118 | - | dirty = &r | |||
| 119 | - | } | |||
| 120 | - | q.signalIfWaitingLocked() | |||
| 121 | - | return key, dirty | |||
| 122 | - | } | |||
| 123 | - | ||||
| 124 | - | func (q *mergeQueue) signalIfWaitingLocked() { | |||
| 125 | - | if len(q.order) > 0 || len(q.dirtyOrder) > 0 { | |||
| 126 | - | q.signal() // let another worker take the rest | |||
| 127 | - | } | |||
| 128 | - | } | |||
| 129 | - | ||||
| 130 | - | // tryRun marks k running for a rescan; when it already runs it asks for | |||
| 131 | - | // another pass instead and reports false. | |||
| 132 | - | func (q *mergeQueue) tryRun(k mergeKey) bool { | |||
| 133 | - | q.mu.Lock() | |||
| 134 | - | defer q.mu.Unlock() | |||
| 135 | - | if q.running[k] { | |||
| 136 | - | q.again[k] = true | |||
| 137 | - | return false | |||
| 138 | - | } | |||
| 139 | - | delete(q.pending, k) // a pending check is covered by this run | |||
| 140 | - | q.running[k] = true | |||
| 141 | - | return true | |||
| 142 | - | } | |||
| 143 | - | ||||
| 144 | - | // finish ends a pass of a running key; it reports whether another is due. | |||
| 145 | - | func (q *mergeQueue) finish(k mergeKey) (again bool) { | |||
| 146 | - | q.mu.Lock() | |||
| 147 | - | defer q.mu.Unlock() | |||
| 148 | - | if q.again[k] { | |||
| 149 | - | delete(q.again, k) | |||
| 150 | - | return true | |||
| 151 | - | } | |||
| 152 | - | delete(q.running, k) | |||
| 153 | - | return false | |||
| 154 | - | } | |||
| 155 | - | ||||
| 156 | - | // markSeen records that a reader was told "checking" for k. | |||
| 157 | - | func (q *mergeQueue) markSeen(k mergeKey) { | |||
| 158 | - | q.mu.Lock() | |||
| 159 | - | q.seen[k] = true | |||
| 160 | - | q.mu.Unlock() | |||
| 161 | - | } | |||
| 162 | - | ||||
| 163 | - | // takeSeen reports and clears the seen flag. | |||
| 164 | - | func (q *mergeQueue) takeSeen(k mergeKey) bool { | |||
| 165 | - | q.mu.Lock() | |||
| 166 | - | defer q.mu.Unlock() | |||
| 167 | - | s := q.seen[k] | |||
| 168 | - | delete(q.seen, k) | |||
| 169 | - | return s | |||
| 170 | - | } | |||
D internal/usecase/mergeability_queue_test.go +0 -74
| @@ -1,74 +0,0 @@ | |||
| 1 | - | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | - | ||
| 3 | - | package usecase | |
| 4 | - | ||
| 5 | - | import ( | |
| 6 | - | "testing" | |
| 7 | - | ||
| 8 | - | "github.com/stretchr/testify/assert" | |
| 9 | - | "github.com/stretchr/testify/require" | |
| 10 | - | ||
| 11 | - | "bnema.dev/ermite/internal/domain" | |
| 12 | - | ) | |
| 13 | - | ||
| 14 | - | func TestMergeQueueNextSkipsAClaimedKey(t *testing.T) { | |
| 15 | - | repo, err := domain.ParseRepoName("proj") | |
| 16 | - | require.NoError(t, err) | |
| 17 | - | k := mergeKey{repo, 1} | |
| 18 | - | q := newMergeQueue(8) | |
| 19 | - | ||
| 20 | - | q.add(k) // pending, in order | |
| 21 | - | require.True(t, q.tryRun(k)) // a rescan claims it: no longer pending, still in order | |
| 22 | - | key, dirty := q.next() | |
| 23 | - | assert.Nil(t, key, "a claimed key is not run again by a worker") | |
| 24 | - | assert.Nil(t, dirty) | |
| 25 | - | assert.True(t, q.running[k], "the rescan's marker is intact") | |
| 26 | - | ||
| 27 | - | // Queued again while it runs: one more pass, still no concurrent run. | |
| 28 | - | q.add(k) | |
| 29 | - | key, _ = q.next() | |
| 30 | - | assert.Nil(t, key) | |
| 31 | - | assert.True(t, q.finish(k), "another pass is due") | |
| 32 | - | assert.False(t, q.finish(k)) | |
| 33 | - | assert.False(t, q.running[k]) | |
| 34 | - | } | |
| 35 | - | ||
| 36 | - | func TestMergeQueueNextMarksAgainForARunningKey(t *testing.T) { | |
| 37 | - | repo, err := domain.ParseRepoName("proj") | |
| 38 | - | require.NoError(t, err) | |
| 39 | - | k := mergeKey{repo, 1} | |
| 40 | - | q := newMergeQueue(8) | |
| 41 | - | ||
| 42 | - | // A stale order entry for a key that is pending again and running. | |
| 43 | - | q.pending[k], q.order, q.running[k] = true, []mergeKey{k}, true | |
| 44 | - | key, _ := q.next() | |
| 45 | - | assert.Nil(t, key) | |
| 46 | - | assert.True(t, q.again[k]) | |
| 47 | - | assert.False(t, q.pending[k]) | |
| 48 | - | } | |
| 49 | - | ||
| 50 | - | func TestMergeQueueNextServicesADirtyRepoUnderSustainedRefill(t *testing.T) { | |
| 51 | - | repo, err := domain.ParseRepoName("proj") | |
| 52 | - | require.NoError(t, err) | |
| 53 | - | q := newMergeQueue(2) | |
| 54 | - | q.markDirty(repo) | |
| 55 | - | n := int64(0) | |
| 56 | - | refill := func() { | |
| 57 | - | for range 2 { | |
| 58 | - | n++ | |
| 59 | - | q.add(mergeKey{repo, n}) | |
| 60 | - | } | |
| 61 | - | } | |
| 62 | - | refill() | |
| 63 | - | for range 3 { | |
| 64 | - | key, dirty := q.next() | |
| 65 | - | if dirty != nil { | |
| 66 | - | assert.Equal(t, repo, *dirty) | |
| 67 | - | return | |
| 68 | - | } | |
| 69 | - | require.NotNil(t, key) | |
| 70 | - | q.finish(*key) | |
| 71 | - | refill() | |
| 72 | - | } | |
| 73 | - | t.Fatal("the dirty repository starved") | |
| 74 | - | } | |
| @@ -1,74 +0,0 @@ | |||||
| 1 | - | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | - | ||||
| 3 | - | package usecase | |||
| 4 | - | ||||
| 5 | - | import ( | |||
| 6 | - | "testing" | |||
| 7 | - | ||||
| 8 | - | "github.com/stretchr/testify/assert" | |||
| 9 | - | "github.com/stretchr/testify/require" | |||
| 10 | - | ||||
| 11 | - | "bnema.dev/ermite/internal/domain" | |||
| 12 | - | ) | |||
| 13 | - | ||||
| 14 | - | func TestMergeQueueNextSkipsAClaimedKey(t *testing.T) { | |||
| 15 | - | repo, err := domain.ParseRepoName("proj") | |||
| 16 | - | require.NoError(t, err) | |||
| 17 | - | k := mergeKey{repo, 1} | |||
| 18 | - | q := newMergeQueue(8) | |||
| 19 | - | ||||
| 20 | - | q.add(k) // pending, in order | |||
| 21 | - | require.True(t, q.tryRun(k)) // a rescan claims it: no longer pending, still in order | |||
| 22 | - | key, dirty := q.next() | |||
| 23 | - | assert.Nil(t, key, "a claimed key is not run again by a worker") | |||
| 24 | - | assert.Nil(t, dirty) | |||
| 25 | - | assert.True(t, q.running[k], "the rescan's marker is intact") | |||
| 26 | - | ||||
| 27 | - | // Queued again while it runs: one more pass, still no concurrent run. | |||
| 28 | - | q.add(k) | |||
| 29 | - | key, _ = q.next() | |||
| 30 | - | assert.Nil(t, key) | |||
| 31 | - | assert.True(t, q.finish(k), "another pass is due") | |||
| 32 | - | assert.False(t, q.finish(k)) | |||
| 33 | - | assert.False(t, q.running[k]) | |||
| 34 | - | } | |||
| 35 | - | ||||
| 36 | - | func TestMergeQueueNextMarksAgainForARunningKey(t *testing.T) { | |||
| 37 | - | repo, err := domain.ParseRepoName("proj") | |||
| 38 | - | require.NoError(t, err) | |||
| 39 | - | k := mergeKey{repo, 1} | |||
| 40 | - | q := newMergeQueue(8) | |||
| 41 | - | ||||
| 42 | - | // A stale order entry for a key that is pending again and running. | |||
| 43 | - | q.pending[k], q.order, q.running[k] = true, []mergeKey{k}, true | |||
| 44 | - | key, _ := q.next() | |||
| 45 | - | assert.Nil(t, key) | |||
| 46 | - | assert.True(t, q.again[k]) | |||
| 47 | - | assert.False(t, q.pending[k]) | |||
| 48 | - | } | |||
| 49 | - | ||||
| 50 | - | func TestMergeQueueNextServicesADirtyRepoUnderSustainedRefill(t *testing.T) { | |||
| 51 | - | repo, err := domain.ParseRepoName("proj") | |||
| 52 | - | require.NoError(t, err) | |||
| 53 | - | q := newMergeQueue(2) | |||
| 54 | - | q.markDirty(repo) | |||
| 55 | - | n := int64(0) | |||
| 56 | - | refill := func() { | |||
| 57 | - | for range 2 { | |||
| 58 | - | n++ | |||
| 59 | - | q.add(mergeKey{repo, n}) | |||
| 60 | - | } | |||
| 61 | - | } | |||
| 62 | - | refill() | |||
| 63 | - | for range 3 { | |||
| 64 | - | key, dirty := q.next() | |||
| 65 | - | if dirty != nil { | |||
| 66 | - | assert.Equal(t, repo, *dirty) | |||
| 67 | - | return | |||
| 68 | - | } | |||
| 69 | - | require.NotNil(t, key) | |||
| 70 | - | q.finish(*key) | |||
| 71 | - | refill() | |||
| 72 | - | } | |||
| 73 | - | t.Fatal("the dirty repository starved") | |||
| 74 | - | } | |||
A internal/usecase/mergeability_reconcile_test.go +95 -0
| @@ -0,0 +1,95 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package usecase | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "context" | |
| 7 | + | "sync/atomic" | |
| 8 | + | "testing" | |
| 9 | + | "time" | |
| 10 | + | ||
| 11 | + | "github.com/stretchr/testify/assert" | |
| 12 | + | "github.com/stretchr/testify/mock" | |
| 13 | + | "github.com/stretchr/testify/require" | |
| 14 | + | ||
| 15 | + | "bnema.dev/ermite/internal/domain" | |
| 16 | + | "bnema.dev/ermite/internal/ports/mocks" | |
| 17 | + | ) | |
| 18 | + | ||
| 19 | + | type mergeTestClock struct{} | |
| 20 | + | ||
| 21 | + | func (mergeTestClock) Now() time.Time { return time.Time{} } | |
| 22 | + | ||
| 23 | + | func TestMergeabilityBackoffIsBounded(t *testing.T) { | |
| 24 | + | assert.Equal(t, time.Second, mergeBackoff(1)) | |
| 25 | + | assert.Equal(t, 2*time.Second, mergeBackoff(2)) | |
| 26 | + | assert.Equal(t, 4*time.Second, mergeBackoff(3)) | |
| 27 | + | assert.Equal(t, time.Minute, mergeBackoff(20)) | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | func TestMergeabilityReadMarksTheRepoDirty(t *testing.T) { | |
| 31 | + | repo, err := domain.ParseRepoName("proj") | |
| 32 | + | require.NoError(t, err) | |
| 33 | + | oid, err := domain.ParseObjectID("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") | |
| 34 | + | require.NoError(t, err) | |
| 35 | + | store := mocks.NewMergeRequestStore(t) | |
| 36 | + | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | |
| 37 | + | svc := NewMergeability(context.Background(), MergeabilityConfig{Store: store}) | |
| 38 | + | defer svc.Close(context.Background()) | |
| 39 | + | mr := domain.MergeRequest{Repo: repo, Number: 7, HeadID: oid} | |
| 40 | + | assert.Equal(t, domain.MergeabilityChecking, svc.Read(context.Background(), mr, oid).State) | |
| 41 | + | // Another MR in the same repository is the same unit of work. | |
| 42 | + | mr.Number = 8 | |
| 43 | + | mr.HeadID = domain.ObjectID{} | |
| 44 | + | svc.Read(context.Background(), mr, oid) | |
| 45 | + | got, found := svc.next() | |
| 46 | + | require.True(t, found) | |
| 47 | + | assert.Equal(t, repo, got) | |
| 48 | + | _, found = svc.next() | |
| 49 | + | assert.False(t, found) | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | func TestMergeabilityOnePassPerRepoAtATimeAndAgain(t *testing.T) { | |
| 53 | + | repo, err := domain.ParseRepoName("proj") | |
| 54 | + | require.NoError(t, err) | |
| 55 | + | other, err := domain.ParseRepoName("other") | |
| 56 | + | require.NoError(t, err) | |
| 57 | + | meta, store := mocks.NewRepoMetaStore(t), mocks.NewMergeRequestStore(t) | |
| 58 | + | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |
| 59 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo}, nil) | |
| 60 | + | meta.EXPECT().Get(mock.Anything, other).Return(domain.Repo{Name: other}, nil) | |
| 61 | + | entered, release, otherPassed := make(chan struct{}, 4), make(chan struct{}), make(chan struct{}) | |
| 62 | + | var passes, inflight, peak atomic.Int32 | |
| 63 | + | store.EXPECT().ListOpen(mock.Anything, repo).RunAndReturn(func(context.Context, domain.RepoName) ([]domain.MergeRequest, error) { | |
| 64 | + | n := inflight.Add(1) | |
| 65 | + | if n > peak.Load() { | |
| 66 | + | peak.Store(n) | |
| 67 | + | } | |
| 68 | + | pass := passes.Add(1) | |
| 69 | + | entered <- struct{}{} | |
| 70 | + | if pass == 1 { | |
| 71 | + | <-release | |
| 72 | + | } | |
| 73 | + | inflight.Add(-1) | |
| 74 | + | return nil, nil | |
| 75 | + | }) | |
| 76 | + | store.EXPECT().ListOpen(mock.Anything, other).RunAndReturn(func(context.Context, domain.RepoName) ([]domain.MergeRequest, error) { | |
| 77 | + | close(otherPassed) | |
| 78 | + | return nil, nil | |
| 79 | + | }) | |
| 80 | + | svc := NewMergeability(context.Background(), MergeabilityConfig{Store: store, Meta: meta, Workers: 2}) | |
| 81 | + | svc.Start() | |
| 82 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: repo}) | |
| 83 | + | <-entered | |
| 84 | + | for range 50 { | |
| 85 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: repo}) | |
| 86 | + | } | |
| 87 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: other}) | |
| 88 | + | <-otherPassed // the other worker has serviced all work eligible meanwhile | |
| 89 | + | assert.EqualValues(t, 1, passes.Load()) | |
| 90 | + | close(release) | |
| 91 | + | <-entered | |
| 92 | + | svc.Close(context.Background()) | |
| 93 | + | assert.EqualValues(t, 2, passes.Load(), "all triggers during a pass coalesce into exactly one more pass") | |
| 94 | + | assert.EqualValues(t, 1, peak.Load()) | |
| 95 | + | } | |
| @@ -0,0 +1,95 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package usecase | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "context" | |||
| 7 | + | "sync/atomic" | |||
| 8 | + | "testing" | |||
| 9 | + | "time" | |||
| 10 | + | ||||
| 11 | + | "github.com/stretchr/testify/assert" | |||
| 12 | + | "github.com/stretchr/testify/mock" | |||
| 13 | + | "github.com/stretchr/testify/require" | |||
| 14 | + | ||||
| 15 | + | "bnema.dev/ermite/internal/domain" | |||
| 16 | + | "bnema.dev/ermite/internal/ports/mocks" | |||
| 17 | + | ) | |||
| 18 | + | ||||
| 19 | + | type mergeTestClock struct{} | |||
| 20 | + | ||||
| 21 | + | func (mergeTestClock) Now() time.Time { return time.Time{} } | |||
| 22 | + | ||||
| 23 | + | func TestMergeabilityBackoffIsBounded(t *testing.T) { | |||
| 24 | + | assert.Equal(t, time.Second, mergeBackoff(1)) | |||
| 25 | + | assert.Equal(t, 2*time.Second, mergeBackoff(2)) | |||
| 26 | + | assert.Equal(t, 4*time.Second, mergeBackoff(3)) | |||
| 27 | + | assert.Equal(t, time.Minute, mergeBackoff(20)) | |||
| 28 | + | } | |||
| 29 | + | ||||
| 30 | + | func TestMergeabilityReadMarksTheRepoDirty(t *testing.T) { | |||
| 31 | + | repo, err := domain.ParseRepoName("proj") | |||
| 32 | + | require.NoError(t, err) | |||
| 33 | + | oid, err := domain.ParseObjectID("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") | |||
| 34 | + | require.NoError(t, err) | |||
| 35 | + | store := mocks.NewMergeRequestStore(t) | |||
| 36 | + | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | |||
| 37 | + | svc := NewMergeability(context.Background(), MergeabilityConfig{Store: store}) | |||
| 38 | + | defer svc.Close(context.Background()) | |||
| 39 | + | mr := domain.MergeRequest{Repo: repo, Number: 7, HeadID: oid} | |||
| 40 | + | assert.Equal(t, domain.MergeabilityChecking, svc.Read(context.Background(), mr, oid).State) | |||
| 41 | + | // Another MR in the same repository is the same unit of work. | |||
| 42 | + | mr.Number = 8 | |||
| 43 | + | mr.HeadID = domain.ObjectID{} | |||
| 44 | + | svc.Read(context.Background(), mr, oid) | |||
| 45 | + | got, found := svc.next() | |||
| 46 | + | require.True(t, found) | |||
| 47 | + | assert.Equal(t, repo, got) | |||
| 48 | + | _, found = svc.next() | |||
| 49 | + | assert.False(t, found) | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | func TestMergeabilityOnePassPerRepoAtATimeAndAgain(t *testing.T) { | |||
| 53 | + | repo, err := domain.ParseRepoName("proj") | |||
| 54 | + | require.NoError(t, err) | |||
| 55 | + | other, err := domain.ParseRepoName("other") | |||
| 56 | + | require.NoError(t, err) | |||
| 57 | + | meta, store := mocks.NewRepoMetaStore(t), mocks.NewMergeRequestStore(t) | |||
| 58 | + | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |||
| 59 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo}, nil) | |||
| 60 | + | meta.EXPECT().Get(mock.Anything, other).Return(domain.Repo{Name: other}, nil) | |||
| 61 | + | entered, release, otherPassed := make(chan struct{}, 4), make(chan struct{}), make(chan struct{}) | |||
| 62 | + | var passes, inflight, peak atomic.Int32 | |||
| 63 | + | store.EXPECT().ListOpen(mock.Anything, repo).RunAndReturn(func(context.Context, domain.RepoName) ([]domain.MergeRequest, error) { | |||
| 64 | + | n := inflight.Add(1) | |||
| 65 | + | if n > peak.Load() { | |||
| 66 | + | peak.Store(n) | |||
| 67 | + | } | |||
| 68 | + | pass := passes.Add(1) | |||
| 69 | + | entered <- struct{}{} | |||
| 70 | + | if pass == 1 { | |||
| 71 | + | <-release | |||
| 72 | + | } | |||
| 73 | + | inflight.Add(-1) | |||
| 74 | + | return nil, nil | |||
| 75 | + | }) | |||
| 76 | + | store.EXPECT().ListOpen(mock.Anything, other).RunAndReturn(func(context.Context, domain.RepoName) ([]domain.MergeRequest, error) { | |||
| 77 | + | close(otherPassed) | |||
| 78 | + | return nil, nil | |||
| 79 | + | }) | |||
| 80 | + | svc := NewMergeability(context.Background(), MergeabilityConfig{Store: store, Meta: meta, Workers: 2}) | |||
| 81 | + | svc.Start() | |||
| 82 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: repo}) | |||
| 83 | + | <-entered | |||
| 84 | + | for range 50 { | |||
| 85 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: repo}) | |||
| 86 | + | } | |||
| 87 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: other}) | |||
| 88 | + | <-otherPassed // the other worker has serviced all work eligible meanwhile | |||
| 89 | + | assert.EqualValues(t, 1, passes.Load()) | |||
| 90 | + | close(release) | |||
| 91 | + | <-entered | |||
| 92 | + | svc.Close(context.Background()) | |||
| 93 | + | assert.EqualValues(t, 2, passes.Load(), "all triggers during a pass coalesce into exactly one more pass") | |||
| 94 | + | assert.EqualValues(t, 1, peak.Load()) | |||
| 95 | + | } | |||
D internal/usecase/mergeability_seen_test.go +0 -47
| @@ -1,47 +0,0 @@ | |||
| 1 | - | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | - | ||
| 3 | - | package usecase | |
| 4 | - | ||
| 5 | - | import ( | |
| 6 | - | "context" | |
| 7 | - | "testing" | |
| 8 | - | "time" | |
| 9 | - | ||
| 10 | - | "github.com/stretchr/testify/assert" | |
| 11 | - | "github.com/stretchr/testify/mock" | |
| 12 | - | "github.com/stretchr/testify/require" | |
| 13 | - | ||
| 14 | - | "bnema.dev/ermite/internal/domain" | |
| 15 | - | "bnema.dev/ermite/internal/ports/mocks" | |
| 16 | - | ) | |
| 17 | - | ||
| 18 | - | type zeroClock struct{} | |
| 19 | - | ||
| 20 | - | func (zeroClock) Now() time.Time { return time.Time{} } | |
| 21 | - | ||
| 22 | - | func TestMergeabilityBackoffIsBounded(t *testing.T) { | |
| 23 | - | assert.Equal(t, time.Second, mergeBackoff(1)) | |
| 24 | - | assert.Equal(t, 2*time.Second, mergeBackoff(2)) | |
| 25 | - | assert.Equal(t, 4*time.Second, mergeBackoff(3)) | |
| 26 | - | assert.Equal(t, time.Minute, mergeBackoff(20)) | |
| 27 | - | } | |
| 28 | - | ||
| 29 | - | func TestMergeabilityClosingForgetsTheSeenKey(t *testing.T) { | |
| 30 | - | repo, err := domain.ParseRepoName("proj") | |
| 31 | - | require.NoError(t, err) | |
| 32 | - | store := mocks.NewMergeRequestStore(t) | |
| 33 | - | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | |
| 34 | - | store.EXPECT().DeleteMergeability(mock.Anything, repo, int64(7)).Return(nil) | |
| 35 | - | svc := NewMergeability(context.Background(), MergeabilityConfig{Store: store, Clock: zeroClock{}}) | |
| 36 | - | t.Cleanup(func() { svc.Close(context.Background()) }) | |
| 37 | - | ||
| 38 | - | mr := domain.MergeRequest{Repo: repo, Number: 7, State: domain.MRStateOpen} | |
| 39 | - | oid, err := domain.ParseObjectID("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") | |
| 40 | - | require.NoError(t, err) | |
| 41 | - | mr.HeadID = oid | |
| 42 | - | svc.Read(context.Background(), mr, oid) | |
| 43 | - | require.Len(t, svc.q.seen, 1) | |
| 44 | - | ||
| 45 | - | svc.OnClosed(context.Background(), mr) | |
| 46 | - | assert.Empty(t, svc.q.seen) | |
| 47 | - | } | |
| @@ -1,47 +0,0 @@ | |||||
| 1 | - | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | - | ||||
| 3 | - | package usecase | |||
| 4 | - | ||||
| 5 | - | import ( | |||
| 6 | - | "context" | |||
| 7 | - | "testing" | |||
| 8 | - | "time" | |||
| 9 | - | ||||
| 10 | - | "github.com/stretchr/testify/assert" | |||
| 11 | - | "github.com/stretchr/testify/mock" | |||
| 12 | - | "github.com/stretchr/testify/require" | |||
| 13 | - | ||||
| 14 | - | "bnema.dev/ermite/internal/domain" | |||
| 15 | - | "bnema.dev/ermite/internal/ports/mocks" | |||
| 16 | - | ) | |||
| 17 | - | ||||
| 18 | - | type zeroClock struct{} | |||
| 19 | - | ||||
| 20 | - | func (zeroClock) Now() time.Time { return time.Time{} } | |||
| 21 | - | ||||
| 22 | - | func TestMergeabilityBackoffIsBounded(t *testing.T) { | |||
| 23 | - | assert.Equal(t, time.Second, mergeBackoff(1)) | |||
| 24 | - | assert.Equal(t, 2*time.Second, mergeBackoff(2)) | |||
| 25 | - | assert.Equal(t, 4*time.Second, mergeBackoff(3)) | |||
| 26 | - | assert.Equal(t, time.Minute, mergeBackoff(20)) | |||
| 27 | - | } | |||
| 28 | - | ||||
| 29 | - | func TestMergeabilityClosingForgetsTheSeenKey(t *testing.T) { | |||
| 30 | - | repo, err := domain.ParseRepoName("proj") | |||
| 31 | - | require.NoError(t, err) | |||
| 32 | - | store := mocks.NewMergeRequestStore(t) | |||
| 33 | - | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | |||
| 34 | - | store.EXPECT().DeleteMergeability(mock.Anything, repo, int64(7)).Return(nil) | |||
| 35 | - | svc := NewMergeability(context.Background(), MergeabilityConfig{Store: store, Clock: zeroClock{}}) | |||
| 36 | - | t.Cleanup(func() { svc.Close(context.Background()) }) | |||
| 37 | - | ||||
| 38 | - | mr := domain.MergeRequest{Repo: repo, Number: 7, State: domain.MRStateOpen} | |||
| 39 | - | oid, err := domain.ParseObjectID("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") | |||
| 40 | - | require.NoError(t, err) | |||
| 41 | - | mr.HeadID = oid | |||
| 42 | - | svc.Read(context.Background(), mr, oid) | |||
| 43 | - | require.Len(t, svc.q.seen, 1) | |||
| 44 | - | ||||
| 45 | - | svc.OnClosed(context.Background(), mr) | |||
| 46 | - | assert.Empty(t, svc.q.seen) | |||
| 47 | - | } | |||
M internal/usecase/mergeability_test.go +86 -243
| @@ -5,7 +5,6 @@ package usecase_test | |||
| 5 | 5 | import ( | |
| 6 | 6 | "context" | |
| 7 | 7 | "errors" | |
| 8 | - | "runtime" | |
| 9 | 8 | "strconv" | |
| 10 | 9 | "strings" | |
| 11 | 10 | "sync/atomic" | |
| @@ -30,8 +29,9 @@ func fakeOID(t *testing.T, c string) domain.ObjectID { | |||
| 30 | 29 | // mergeRig runs the mergeability service over the real adapters of mrRig. | |
| 31 | 30 | type mergeRig struct { | |
| 32 | 31 | *mrRig | |
| 33 | - | svc *usecase.Mergeability | |
| 34 | - | mev <-chan domain.Event | |
| 32 | + | svc *usecase.Mergeability | |
| 33 | + | mev <-chan domain.Event | |
| 34 | + | history []domain.MergeRequestMergeability | |
| 35 | 35 | } | |
| 36 | 36 | ||
| 37 | 37 | func newMergeRig(t *testing.T) *mergeRig { | |
| @@ -56,17 +56,30 @@ func (r *mergeRig) record(n int64) (domain.Mergeability, error) { | |||
| 56 | 56 | func (r *mergeRig) waitState(n int64, want domain.MergeabilityState, target, head domain.ObjectID) domain.Mergeability { | |
| 57 | 57 | r.t.Helper() | |
| 58 | 58 | var m domain.Mergeability | |
| 59 | - | require.Eventually(r.t, func() bool { | |
| 59 | + | deadline := time.After(10 * time.Second) | |
| 60 | + | for { | |
| 60 | 61 | var err error | |
| 61 | 62 | m, err = r.record(n) | |
| 62 | - | return err == nil && m.State == want && m.IsFor(target, head) | |
| 63 | - | }, 10*time.Second, 10*time.Millisecond, "mergeability of !%d did not become %s", n, want) | |
| 63 | + | if err == nil && m.State == want && m.IsFor(target, head) { | |
| 64 | + | break | |
| 65 | + | } | |
| 66 | + | select { | |
| 67 | + | case ev := <-r.mev: | |
| 68 | + | if e, ok := ev.(domain.MergeRequestMergeability); ok { | |
| 69 | + | r.history = append(r.history, e) | |
| 70 | + | } | |
| 71 | + | case <-deadline: | |
| 72 | + | r.t.Fatalf("mergeability of !%d did not become %s", n, want) | |
| 73 | + | } | |
| 74 | + | } | |
| 75 | + | ||
| 64 | 76 | return m | |
| 65 | 77 | } | |
| 66 | 78 | ||
| 67 | 79 | // events returns the mergeability events published so far. | |
| 68 | 80 | func (r *mergeRig) events() []domain.MergeRequestMergeability { | |
| 69 | - | var out []domain.MergeRequestMergeability | |
| 81 | + | out := r.history | |
| 82 | + | r.history = nil | |
| 70 | 83 | for { | |
| 71 | 84 | select { | |
| 72 | 85 | case ev := <-r.mev: | |
| @@ -83,7 +96,8 @@ func (r *mergeRig) events() []domain.MergeRequestMergeability { | |||
| 83 | 96 | // bound: the workers publish after they stored the record the tests wait for. | |
| 84 | 97 | func (r *mergeRig) awaitEvents(n int) []domain.MergeRequestMergeability { | |
| 85 | 98 | r.t.Helper() | |
| 86 | - | var out []domain.MergeRequestMergeability | |
| 99 | + | out := r.history | |
| 100 | + | r.history = nil | |
| 87 | 101 | deadline := time.After(10 * time.Second) | |
| 88 | 102 | for len(out) < n { | |
| 89 | 103 | select { | |
| @@ -153,49 +167,29 @@ func TestMergeabilityReaderIsLazy(t *testing.T) { | |||
| 153 | 167 | assert.Equal(t, domain.MergeabilityUpToDate, page.Mergeability[mr.Number].State) | |
| 154 | 168 | } | |
| 155 | 169 | ||
| 156 | - | func TestMergeabilityTransitionOnlyPublication(t *testing.T) { | |
| 170 | + | func TestMergeabilityPublishesEachSettledResultOnce(t *testing.T) { | |
| 157 | 171 | r := newMergeRig(t) | |
| 158 | 172 | r.seed() | |
| 159 | 173 | r.feature("feat") | |
| 160 | 174 | mr := r.open("feat") | |
| 161 | - | r.svc.Check(r.repo, mr.Number) | |
| 175 | + | r.svc.ReconcileForTest(r.repo) | |
| 162 | 176 | require.Len(t, r.events(), 1) | |
| 163 | 177 | ||
| 164 | 178 | // Same tips: nothing is computed, nothing is published. | |
| 165 | - | r.svc.Check(r.repo, mr.Number) | |
| 179 | + | r.svc.ReconcileForTest(r.repo) | |
| 166 | 180 | assert.Empty(t, r.events()) | |
| 167 | 181 | ||
| 168 | - | // The target moves without changing the state: a new record but, being the | |
| 169 | - | // same state and counts, no event. | |
| 182 | + | // New tips publish even when the state and counts stay the same. | |
| 170 | 183 | r.sh("checkout", "-q", "feat") | |
| 171 | 184 | r.commit(author, "e.txt", "e\n", "more") | |
| 172 | 185 | r.push("feat") | |
| 173 | - | r.svc.Check(r.repo, mr.Number) | |
| 174 | - | assert.Empty(t, r.events()) | |
| 186 | + | r.svc.ReconcileForTest(r.repo) | |
| 187 | + | assert.Len(t, r.events(), 1) | |
| 175 | 188 | m, err := r.record(mr.Number) | |
| 176 | 189 | require.NoError(t, err) | |
| 177 | 190 | assert.Equal(t, domain.MergeabilityUpToDate, m.State) | |
| 178 | 191 | } | |
| 179 | 192 | ||
| 180 | - | func TestMergeabilityPublishesWhenReadersSawChecking(t *testing.T) { | |
| 181 | - | r := newMergeRig(t) | |
| 182 | - | r.seed() | |
| 183 | - | r.feature("feat") | |
| 184 | - | mr := r.open("feat") | |
| 185 | - | r.svc.Check(r.repo, mr.Number) | |
| 186 | - | require.Len(t, r.events(), 1) | |
| 187 | - | ||
| 188 | - | r.sh("checkout", "-q", "feat") | |
| 189 | - | r.commit(author, "e.txt", "e\n", "more") | |
| 190 | - | r.push("feat") | |
| 191 | - | _, err := r.mrs.Get(r.ctx, root, r.repo, mr.Number, ports.GetMergeRequestOptions{}) // sees "checking" | |
| 192 | - | require.NoError(t, err) | |
| 193 | - | r.svc.Check(r.repo, mr.Number) | |
| 194 | - | got := r.events() | |
| 195 | - | require.Len(t, got, 1) | |
| 196 | - | assert.Equal(t, domain.MergeabilityUpToDate, got[0].Mergeability.State) | |
| 197 | - | } | |
| 198 | - | ||
| 199 | 193 | func TestMergeabilityMergeReevaluatesOtherRequests(t *testing.T) { | |
| 200 | 194 | r := newMergeRig(t) | |
| 201 | 195 | r.svc.Start() | |
| @@ -223,7 +217,7 @@ func TestMergeabilityClosedRequestRecordDeleted(t *testing.T) { | |||
| 223 | 217 | r.seed() | |
| 224 | 218 | r.feature("feat") | |
| 225 | 219 | mr := r.open("feat") | |
| 226 | - | r.svc.Check(r.repo, mr.Number) | |
| 220 | + | r.svc.ReconcileForTest(r.repo) | |
| 227 | 221 | _, err := r.record(mr.Number) | |
| 228 | 222 | require.NoError(t, err) | |
| 229 | 223 | ||
| @@ -231,7 +225,7 @@ func TestMergeabilityClosedRequestRecordDeleted(t *testing.T) { | |||
| 231 | 225 | require.NoError(t, err) | |
| 232 | 226 | _, err = r.record(mr.Number) | |
| 233 | 227 | assert.ErrorIs(t, err, domain.ErrNotFound) | |
| 234 | - | r.svc.Check(r.repo, mr.Number) // a late check writes nothing | |
| 228 | + | r.svc.ReconcileForTest(r.repo) // a late check writes nothing | |
| 235 | 229 | _, err = r.record(mr.Number) | |
| 236 | 230 | assert.ErrorIs(t, err, domain.ErrNotFound) | |
| 237 | 231 | } | |
| @@ -244,7 +238,12 @@ func TestMergeabilityStartupRescanComputesMissingRecords(t *testing.T) { | |||
| 244 | 238 | _, err := r.record(mr.Number) | |
| 245 | 239 | require.ErrorIs(t, err, domain.ErrNotFound) | |
| 246 | 240 | ||
| 247 | - | r.svc.Start() | |
| 241 | + | // A fresh service has received no OnOpened or Read trigger. | |
| 242 | + | svc := usecase.NewMergeability(r.ctx, usecase.MergeabilityConfig{ | |
| 243 | + | Store: r.store.MergeRequests(), Meta: r.store, Git: r.git, Bus: r.bus, Clock: fixedClock{}, | |
| 244 | + | }) | |
| 245 | + | defer svc.Close(context.Background()) | |
| 246 | + | svc.Start() | |
| 248 | 247 | r.waitState(mr.Number, domain.MergeabilityUpToDate, main, head) | |
| 249 | 248 | } | |
| 250 | 249 | ||
| @@ -284,7 +283,7 @@ func newMockRig(t *testing.T) *mockRig { | |||
| 284 | 283 | store: mocks.NewMergeRequestStore(t), meta: mocks.NewRepoMetaStore(t), git: mocks.NewMergeGit(t), bus: mocks.NewEventBus(t), | |
| 285 | 284 | mr: domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen}, | |
| 286 | 285 | } | |
| 287 | - | r.store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(r.mr, nil).Maybe() | |
| 286 | + | r.store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{r.mr}, nil).Maybe() | |
| 288 | 287 | r.store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound).Maybe() | |
| 289 | 288 | r.meta.EXPECT().List(mock.Anything).Return(nil, nil).Maybe() | |
| 290 | 289 | r.meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil).Maybe() | |
| @@ -295,45 +294,18 @@ func newMockRig(t *testing.T) *mockRig { | |||
| 295 | 294 | return r | |
| 296 | 295 | } | |
| 297 | 296 | ||
| 298 | - | func TestMergeabilityStaleResultIsDiscarded(t *testing.T) { | |
| 299 | - | r := newMockRig(t) | |
| 300 | - | repo := r.mr.Repo | |
| 301 | - | head, t1, t2 := fakeOID(t, "a"), fakeOID(t, "b"), fakeOID(t, "c") | |
| 302 | - | var moved atomic.Bool | |
| 303 | - | r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |
| 304 | - | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").RunAndReturn( | |
| 305 | - | func(context.Context, domain.RepoName, string) (domain.ObjectID, error) { | |
| 306 | - | if moved.Load() { | |
| 307 | - | return t2, nil | |
| 308 | - | } | |
| 309 | - | return t1, nil | |
| 310 | - | }) | |
| 311 | - | // The target moves while the first computation runs. | |
| 312 | - | r.git.EXPECT().IsAncestor(mock.Anything, repo, mock.Anything, head).RunAndReturn( | |
| 313 | - | func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID) (bool, error) { | |
| 314 | - | moved.Store(true) | |
| 315 | - | return true, nil | |
| 316 | - | }) | |
| 317 | - | var puts []domain.Mergeability | |
| 318 | - | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn( | |
| 319 | - | func(_ context.Context, m domain.Mergeability) error { puts = append(puts, m); return nil }) | |
| 320 | - | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | |
| 321 | - | ||
| 322 | - | r.svc.Check(repo, 7) | |
| 323 | - | require.Len(t, puts, 1, "the result for the old tip is dropped, the one for the new tip is stored") | |
| 324 | - | assert.Equal(t, t2, puts[0].TargetID) | |
| 325 | - | } | |
| 326 | - | ||
| 327 | 297 | func TestMergeabilityDedupsTriggers(t *testing.T) { | |
| 328 | 298 | r := newMockRig(t) | |
| 329 | 299 | repo := r.mr.Repo | |
| 330 | 300 | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |
| 331 | 301 | r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |
| 332 | 302 | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |
| 303 | + | computed := make(chan struct{}, 1) | |
| 333 | 304 | var computes atomic.Int32 | |
| 334 | 305 | r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).RunAndReturn( | |
| 335 | 306 | func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID) (bool, error) { | |
| 336 | 307 | computes.Add(1) | |
| 308 | + | computed <- struct{}{} | |
| 337 | 309 | return true, nil | |
| 338 | 310 | }) | |
| 339 | 311 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | |
| @@ -343,7 +315,7 @@ func TestMergeabilityDedupsTriggers(t *testing.T) { | |||
| 343 | 315 | r.svc.OnOpened(context.Background(), r.mr) // queued before any worker runs | |
| 344 | 316 | } | |
| 345 | 317 | r.svc.Start() | |
| 346 | - | require.Eventually(t, func() bool { return computes.Load() >= 1 }, 5*time.Second, 5*time.Millisecond) | |
| 318 | + | <-computed | |
| 347 | 319 | r.svc.Close(context.Background()) // the workers have each looked at the queue once more | |
| 348 | 320 | assert.EqualValues(t, 1, computes.Load()) | |
| 349 | 321 | } | |
| @@ -356,71 +328,7 @@ func TestMergeabilityNoWriteForClosedRequest(t *testing.T) { | |||
| 356 | 328 | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |
| 357 | 329 | r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(true, nil) | |
| 358 | 330 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(domain.ErrNotFound).Once() // closed meanwhile | |
| 359 | - | r.svc.Check(repo, 7) // no event, no further write | |
| 360 | - | } | |
| 361 | - | ||
| 362 | - | // A reader that saw a stale record marks the request seen and queues it; the | |
| 363 | - | // worker finds the record already current (stored and published by an earlier | |
| 364 | - | // check) and must still publish it, as the reader was told "checking". | |
| 365 | - | func TestMergeabilityFastPathPublishesWhenAReaderSawChecking(t *testing.T) { | |
| 366 | - | repo := name(t, "proj") | |
| 367 | - | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |
| 368 | - | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen, HeadID: head} | |
| 369 | - | rec := domain.Mergeability{Repo: repo, Number: 7, State: domain.MergeabilityUpToDate, TargetID: tip, HeadID: head, CheckedAt: now} | |
| 370 | - | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | |
| 371 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil) | |
| 372 | - | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound).Once() // the reader's read: stale | |
| 373 | - | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(rec, nil) // the worker has stored it since | |
| 374 | - | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil) | |
| 375 | - | git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |
| 376 | - | git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |
| 377 | - | var published []domain.Event | |
| 378 | - | bus.EXPECT().Publish(mock.Anything, mock.Anything).Run(func(_ context.Context, ev domain.Event) { published = append(published, ev) }).Return().Once() | |
| 379 | - | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}}) | |
| 380 | - | t.Cleanup(func() { svc.Close(context.Background()) }) | |
| 381 | - | ||
| 382 | - | got := svc.Read(context.Background(), mr, tip) | |
| 383 | - | assert.Equal(t, domain.MergeabilityChecking, got.State) | |
| 384 | - | svc.Check(repo, 7) | |
| 385 | - | require.Len(t, published, 1, "the current record is published for the reader") | |
| 386 | - | assert.Equal(t, domain.MergeabilityUpToDate, published[0].(domain.MergeRequestMergeability).Mergeability.State) | |
| 387 | - | ||
| 388 | - | svc.Check(repo, 7) // nobody saw "checking" since: nothing more | |
| 389 | - | assert.Len(t, published, 1) | |
| 390 | - | } | |
| 391 | - | ||
| 392 | - | // A rescan claims a key that is also pending; a worker must then not run it | |
| 393 | - | // concurrently (GO-001). | |
| 394 | - | func TestMergeabilityRescanAndWorkersNeverRunAKeyConcurrently(t *testing.T) { | |
| 395 | - | r := newMockRig(t) | |
| 396 | - | repo := r.mr.Repo | |
| 397 | - | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |
| 398 | - | r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |
| 399 | - | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |
| 400 | - | var inflight, peak atomic.Int32 | |
| 401 | - | entered, release := make(chan struct{}, 8), make(chan struct{}) | |
| 402 | - | r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).RunAndReturn( | |
| 403 | - | func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID) (bool, error) { | |
| 404 | - | n := inflight.Add(1) | |
| 405 | - | for p := peak.Load(); n > p && !peak.CompareAndSwap(p, n); p = peak.Load() { | |
| 406 | - | } | |
| 407 | - | entered <- struct{}{} | |
| 408 | - | <-release | |
| 409 | - | inflight.Add(-1) | |
| 410 | - | return true, nil | |
| 411 | - | }) | |
| 412 | - | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | |
| 413 | - | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | |
| 414 | - | ||
| 415 | - | r.svc.OnOpened(context.Background(), r.mr) // pending | |
| 416 | - | done := make(chan struct{}) | |
| 417 | - | go func() { r.svc.Check(repo, 7); close(done) }() // claims it, as a rescan does | |
| 418 | - | <-entered | |
| 419 | - | r.svc.Start() // two workers | |
| 420 | - | r.svc.Close(context.Background()) | |
| 421 | - | close(release) | |
| 422 | - | <-done | |
| 423 | - | assert.EqualValues(t, 1, peak.Load(), "one check of the key at a time") | |
| 331 | + | r.svc.ReconcileForTest(repo) // no event, no further write | |
| 424 | 332 | } | |
| 425 | 333 | ||
| 426 | 334 | func TestMergeabilityBoundsConflictPathsAndBehind(t *testing.T) { | |
| @@ -440,7 +348,7 @@ func TestMergeabilityBoundsConflictPathsAndBehind(t *testing.T) { | |||
| 440 | 348 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn( | |
| 441 | 349 | func(_ context.Context, m domain.Mergeability) error { put = m; return nil }) | |
| 442 | 350 | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | |
| 443 | - | r.svc.Check(repo, 7) | |
| 351 | + | r.svc.ReconcileForTest(repo) | |
| 444 | 352 | assert.Equal(t, domain.MergeabilityConflict, put.State) | |
| 445 | 353 | assert.Len(t, put.ConflictPaths, domain.MaxConflictPaths) | |
| 446 | 354 | assert.True(t, put.ConflictPathsTruncated) | |
| @@ -458,7 +366,7 @@ func TestMergeabilityBoundsConflictPathsAndBehind(t *testing.T) { | |||
| 458 | 366 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn( | |
| 459 | 367 | func(_ context.Context, m domain.Mergeability) error { put = m; return nil }) | |
| 460 | 368 | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | |
| 461 | - | r.svc.Check(repo, 7) | |
| 369 | + | r.svc.ReconcileForTest(repo) | |
| 462 | 370 | assert.Equal(t, domain.MaxBehindCount, put.Behind) | |
| 463 | 371 | assert.True(t, put.BehindCapped) | |
| 464 | 372 | }) | |
| @@ -471,10 +379,9 @@ func TestMergeabilityRetriesATransientFailureWithBackoff(t *testing.T) { | |||
| 471 | 379 | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |
| 472 | 380 | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | |
| 473 | 381 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | |
| 474 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil) | |
| 382 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil) | |
| 475 | 383 | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | |
| 476 | 384 | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | |
| 477 | - | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil).Once() | |
| 478 | 385 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |
| 479 | 386 | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil) | |
| 480 | 387 | var fails atomic.Int32 | |
| @@ -515,7 +422,8 @@ func TestMergeabilityCloseStopsPendingRetries(t *testing.T) { | |||
| 515 | 422 | repo := name(t, "proj") | |
| 516 | 423 | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | |
| 517 | 424 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | |
| 518 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(domain.MergeRequest{}, errors.New("db down")) | |
| 425 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return(nil, errors.New("db down")) | |
| 426 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo}, nil) | |
| 519 | 427 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |
| 520 | 428 | asked := make(chan time.Duration, 4) | |
| 521 | 429 | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |
| @@ -537,7 +445,7 @@ func TestMergeabilityWorkersOutliveTheParentContext(t *testing.T) { | |||
| 537 | 445 | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |
| 538 | 446 | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | |
| 539 | 447 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | |
| 540 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil) | |
| 448 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil) | |
| 541 | 449 | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | |
| 542 | 450 | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | |
| 543 | 451 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |
| @@ -560,27 +468,33 @@ func TestMergeabilityWorkersOutliveTheParentContext(t *testing.T) { | |||
| 560 | 468 | // Failures of many requests of 3 repositories schedule at most one retry per | |
| 561 | 469 | // repository on a single scheduler goroutine. | |
| 562 | 470 | func TestMergeabilityRetriesAreBounded(t *testing.T) { | |
| 563 | - | const repos, perRepo = 3, 20 | |
| 564 | - | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | |
| 565 | - | store.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(domain.MergeRequest{}, errors.New("db down")) | |
| 471 | + | const repos = 3 | |
| 472 | + | store, meta := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t) | |
| 473 | + | barrier := name(t, "barrier") | |
| 474 | + | done := make(chan struct{}) | |
| 475 | + | store.EXPECT().ListOpen(mock.Anything, barrier).RunAndReturn(func(context.Context, domain.RepoName) ([]domain.MergeRequest, error) { close(done); return nil, nil }).Once() | |
| 476 | + | store.EXPECT().ListOpen(mock.Anything, mock.Anything).Return(nil, errors.New("db down")) | |
| 477 | + | meta.EXPECT().Get(mock.Anything, mock.Anything).Return(domain.Repo{}, nil) | |
| 566 | 478 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |
| 479 | + | asked := make(chan time.Duration, repos) | |
| 567 | 480 | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |
| 568 | - | Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}, Workers: 1, Queue: 4, | |
| 569 | - | After: func(time.Duration) <-chan time.Time { return make(chan time.Time) }, | |
| 481 | + | Store: store, Meta: meta, Clock: fixedClock{}, Workers: 1, | |
| 482 | + | After: func(d time.Duration) <-chan time.Time { asked <- d; return make(chan time.Time) }, | |
| 570 | 483 | }) | |
| 571 | 484 | svc.Start() | |
| 572 | - | before := runtime.NumGoroutine() | |
| 573 | 485 | for r := range repos { | |
| 574 | 486 | repo := name(t, "proj"+strconv.Itoa(r)) | |
| 575 | - | for n := range perRepo { | |
| 576 | - | svc.Check(repo, int64(n+1)) | |
| 487 | + | for range 20 { | |
| 488 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: repo}) | |
| 577 | 489 | } | |
| 578 | 490 | } | |
| 579 | - | assert.Positive(t, svc.ScheduledRetries()) | |
| 580 | - | assert.LessOrEqual(t, svc.ScheduledRetries(), repos+1) | |
| 581 | - | assert.LessOrEqual(t, svc.RetryAttempts(), repos+1) | |
| 582 | - | assert.LessOrEqual(t, runtime.NumGoroutine(), before, "retries must not start goroutines") | |
| 583 | - | svc.Close(context.Background()) // returns although retries are pending | |
| 491 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: barrier}) | |
| 492 | + | <-done | |
| 493 | + | // The shared scheduler only arms the earliest pending retry. | |
| 494 | + | <-asked | |
| 495 | + | svc.Close(context.Background()) | |
| 496 | + | assert.Equal(t, repos, svc.ScheduledRetries()) | |
| 497 | + | assert.Equal(t, repos, svc.RetryAttempts()) | |
| 584 | 498 | } | |
| 585 | 499 | ||
| 586 | 500 | // retryRig is a service over mocks whose only repository holds request !7, | |
| @@ -592,26 +506,28 @@ type retryRig struct { | |||
| 592 | 506 | failFor atomic.Int32 | |
| 593 | 507 | passes chan struct{} | |
| 594 | 508 | afters chan chan time.Time | |
| 509 | + | barrier chan struct{} | |
| 595 | 510 | } | |
| 596 | 511 | ||
| 597 | 512 | func newRetryRig(t *testing.T) *retryRig { | |
| 598 | 513 | t.Helper() | |
| 599 | - | r := &retryRig{repo: name(t, "proj"), passes: make(chan struct{}, 32), afters: make(chan chan time.Time, 32)} | |
| 514 | + | r := &retryRig{repo: name(t, "proj"), passes: make(chan struct{}, 32), afters: make(chan chan time.Time, 32), barrier: make(chan struct{})} | |
| 600 | 515 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | |
| 601 | 516 | r.failFor.Store(1 << 30) | |
| 602 | - | store.EXPECT().Get(mock.Anything, r.repo, int64(7)).RunAndReturn( | |
| 603 | - | func(context.Context, domain.RepoName, int64) (domain.MergeRequest, error) { | |
| 604 | - | if r.reads.Add(1) <= r.failFor.Load() { | |
| 605 | - | return domain.MergeRequest{}, errors.New("db down") | |
| 606 | - | } | |
| 607 | - | return domain.MergeRequest{}, domain.ErrNotFound | |
| 608 | - | }) | |
| 517 | + | meta.EXPECT().Get(mock.Anything, r.repo).Return(domain.Repo{Name: r.repo}, nil) | |
| 609 | 518 | store.EXPECT().ListOpen(mock.Anything, r.repo).RunAndReturn( | |
| 610 | 519 | func(context.Context, domain.RepoName) ([]domain.MergeRequest, error) { | |
| 611 | - | r.passes <- struct{}{} | |
| 612 | - | return []domain.MergeRequest{{Repo: r.repo, Number: 7}}, nil | |
| 520 | + | defer func() { r.passes <- struct{}{} }() | |
| 521 | + | if r.reads.Add(1) <= r.failFor.Load() { | |
| 522 | + | return nil, errors.New("db down") | |
| 523 | + | } | |
| 524 | + | return nil, nil | |
| 613 | 525 | }) | |
| 614 | - | store.EXPECT().CountOpen(mock.Anything, r.repo).Return(1, nil) | |
| 526 | + | barrier := name(t, "barrier") | |
| 527 | + | meta.EXPECT().Get(mock.Anything, barrier).RunAndReturn(func(context.Context, domain.RepoName) (domain.Repo, error) { | |
| 528 | + | r.barrier <- struct{}{} | |
| 529 | + | return domain.Repo{}, domain.ErrNotFound | |
| 530 | + | }) | |
| 615 | 531 | meta.EXPECT().List(mock.Anything).Return([]domain.Repo{{Name: r.repo}}, nil) | |
| 616 | 532 | r.svc = usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |
| 617 | 533 | Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}, Workers: 1, | |
| @@ -630,8 +546,10 @@ func (r *retryRig) next() { | |||
| 630 | 546 | ||
| 631 | 547 | func (r *retryRig) settled(t *testing.T) { | |
| 632 | 548 | t.Helper() | |
| 633 | - | require.Eventually(t, func() bool { return r.svc.RetryAttempts() == 0 && r.svc.ScheduledRetries() == 0 }, | |
| 634 | - | 10*time.Second, time.Millisecond) | |
| 549 | + | r.svc.OnOpened(context.Background(), domain.MergeRequest{Repo: name(t, "barrier")}) | |
| 550 | + | <-r.barrier | |
| 551 | + | require.Zero(t, r.svc.RetryAttempts()) | |
| 552 | + | require.Zero(t, r.svc.ScheduledRetries()) | |
| 635 | 553 | } | |
| 636 | 554 | ||
| 637 | 555 | // A persistent failure is retried 5 times, then given up with no state kept. | |
| @@ -648,7 +566,7 @@ func TestMergeabilityGivesUpAfterFiveRetries(t *testing.T) { | |||
| 648 | 566 | } | |
| 649 | 567 | ||
| 650 | 568 | // A successful full rescan resets the budget: a later failure gets 5 again. | |
| 651 | - | func TestMergeabilityFullRescanResetsTheRetryBudget(t *testing.T) { | |
| 569 | + | func TestMergeabilitySuccessfulPassForgetsTheRetryBudget(t *testing.T) { | |
| 652 | 570 | r := newRetryRig(t) | |
| 653 | 571 | r.failFor.Store(2) // the first two reads fail, the third succeeds | |
| 654 | 572 | r.svc.Start() | |
| @@ -657,7 +575,8 @@ func TestMergeabilityFullRescanResetsTheRetryBudget(t *testing.T) { | |||
| 657 | 575 | r.next() // success | |
| 658 | 576 | r.settled(t) | |
| 659 | 577 | r.failFor.Store(1 << 30) | |
| 660 | - | r.svc.Check(r.repo, 7) // fails alone: retries the repository | |
| 578 | + | r.svc.OnOpened(context.Background(), domain.MergeRequest{Repo: r.repo}) | |
| 579 | + | <-r.passes | |
| 661 | 580 | for range 5 { | |
| 662 | 581 | r.next() | |
| 663 | 582 | } | |
| @@ -665,79 +584,3 @@ func TestMergeabilityFullRescanResetsTheRetryBudget(t *testing.T) { | |||
| 665 | 584 | assert.Empty(t, r.afters) | |
| 666 | 585 | assert.Empty(t, r.passes) | |
| 667 | 586 | } | |
| 668 | - | ||
| 669 | - | // A transient failure to publish after the store is retried and the event is | |
| 670 | - | // published without any other trigger. | |
| 671 | - | func TestMergeabilityRetriesAFailedPublish(t *testing.T) { | |
| 672 | - | repo := name(t, "proj") | |
| 673 | - | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |
| 674 | - | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | |
| 675 | - | rec := domain.Mergeability{Repo: repo, Number: 7, TargetID: tip, HeadID: head, State: domain.MergeabilityUpToDate} | |
| 676 | - | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | |
| 677 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil) | |
| 678 | - | var gets atomic.Int32 | |
| 679 | - | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).RunAndReturn( | |
| 680 | - | func(context.Context, domain.RepoName, int64) (domain.Mergeability, error) { | |
| 681 | - | if gets.Add(1) == 1 { | |
| 682 | - | return domain.Mergeability{}, domain.ErrNotFound | |
| 683 | - | } | |
| 684 | - | return rec, nil | |
| 685 | - | }) | |
| 686 | - | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil).Once() | |
| 687 | - | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil) | |
| 688 | - | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |
| 689 | - | var metaGets atomic.Int32 | |
| 690 | - | meta.EXPECT().Get(mock.Anything, repo).RunAndReturn( | |
| 691 | - | func(context.Context, domain.RepoName) (domain.Repo, error) { | |
| 692 | - | if metaGets.Add(1) == 1 { | |
| 693 | - | return domain.Repo{}, errors.New("transient") | |
| 694 | - | } | |
| 695 | - | return domain.Repo{Name: repo, CreatedAt: now}, nil | |
| 696 | - | }) | |
| 697 | - | git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |
| 698 | - | git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |
| 699 | - | git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(true, nil) | |
| 700 | - | published := make(chan struct{}) | |
| 701 | - | bus.EXPECT().Publish(mock.Anything, mock.Anything).Run(func(context.Context, domain.Event) { close(published) }).Return().Once() | |
| 702 | - | ||
| 703 | - | afters := make(chan chan time.Time, 4) | |
| 704 | - | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |
| 705 | - | Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}, | |
| 706 | - | After: func(time.Duration) <-chan time.Time { ch := make(chan time.Time, 1); afters <- ch; return ch }, | |
| 707 | - | }) | |
| 708 | - | t.Cleanup(func() { svc.Close(context.Background()) }) | |
| 709 | - | svc.Start() | |
| 710 | - | svc.OnOpened(context.Background(), mr) | |
| 711 | - | <-svcWaiting(afters) | |
| 712 | - | select { | |
| 713 | - | case <-published: | |
| 714 | - | t.Fatal("published although the repository could not be read") | |
| 715 | - | default: | |
| 716 | - | } | |
| 717 | - | stop := fireAll(afters) | |
| 718 | - | defer stop() | |
| 719 | - | <-published | |
| 720 | - | } | |
| 721 | - | ||
| 722 | - | // svcWaiting blocks until the scheduler asked for a timer, and puts it back. | |
| 723 | - | func svcWaiting(afters chan chan time.Time) <-chan struct{} { | |
| 724 | - | ready := make(chan struct{}) | |
| 725 | - | go func() { ch := <-afters; afters <- ch; close(ready) }() | |
| 726 | - | return ready | |
| 727 | - | } | |
| 728 | - | ||
| 729 | - | // fireAll fires every timer the scheduler asks for (stale ones are harmless). | |
| 730 | - | func fireAll(afters chan chan time.Time) (stop func()) { | |
| 731 | - | quit := make(chan struct{}) | |
| 732 | - | go func() { | |
| 733 | - | for { | |
| 734 | - | select { | |
| 735 | - | case ch := <-afters: | |
| 736 | - | ch <- time.Time{} | |
| 737 | - | case <-quit: | |
| 738 | - | return | |
| 739 | - | } | |
| 740 | - | } | |
| 741 | - | }() | |
| 742 | - | return func() { close(quit) } | |
| 743 | - | } | |
| @@ -5,7 +5,6 @@ package usecase_test | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "context" | 6 | "context" | ||
| 7 | "errors" | 7 | "errors" | ||
| 8 | - | "runtime" | |||
| 9 | "strconv" | 8 | "strconv" | ||
| 10 | "strings" | 9 | "strings" | ||
| 11 | "sync/atomic" | 10 | "sync/atomic" | ||
| @@ -30,8 +29,9 @@ func fakeOID(t *testing.T, c string) domain.ObjectID { | |||||
| 30 | // mergeRig runs the mergeability service over the real adapters of mrRig. | 29 | // mergeRig runs the mergeability service over the real adapters of mrRig. | ||
| 31 | type mergeRig struct { | 30 | type mergeRig struct { | ||
| 32 | *mrRig | 31 | *mrRig | ||
| 33 | - | svc *usecase.Mergeability | 32 | + | svc *usecase.Mergeability |
| 34 | - | mev <-chan domain.Event | 33 | + | mev <-chan domain.Event |
| 34 | + | history []domain.MergeRequestMergeability | |||
| 35 | } | 35 | } | ||
| 36 | 36 | ||||
| 37 | func newMergeRig(t *testing.T) *mergeRig { | 37 | func newMergeRig(t *testing.T) *mergeRig { | ||
| @@ -56,17 +56,30 @@ func (r *mergeRig) record(n int64) (domain.Mergeability, error) { | |||||
| 56 | func (r *mergeRig) waitState(n int64, want domain.MergeabilityState, target, head domain.ObjectID) domain.Mergeability { | 56 | func (r *mergeRig) waitState(n int64, want domain.MergeabilityState, target, head domain.ObjectID) domain.Mergeability { | ||
| 57 | r.t.Helper() | 57 | r.t.Helper() | ||
| 58 | var m domain.Mergeability | 58 | var m domain.Mergeability | ||
| 59 | - | require.Eventually(r.t, func() bool { | 59 | + | deadline := time.After(10 * time.Second) |
| 60 | + | for { | |||
| 60 | var err error | 61 | var err error | ||
| 61 | m, err = r.record(n) | 62 | m, err = r.record(n) | ||
| 62 | - | return err == nil && m.State == want && m.IsFor(target, head) | 63 | + | if err == nil && m.State == want && m.IsFor(target, head) { |
| 63 | - | }, 10*time.Second, 10*time.Millisecond, "mergeability of !%d did not become %s", n, want) | 64 | + | break |
| 65 | + | } | |||
| 66 | + | select { | |||
| 67 | + | case ev := <-r.mev: | |||
| 68 | + | if e, ok := ev.(domain.MergeRequestMergeability); ok { | |||
| 69 | + | r.history = append(r.history, e) | |||
| 70 | + | } | |||
| 71 | + | case <-deadline: | |||
| 72 | + | r.t.Fatalf("mergeability of !%d did not become %s", n, want) | |||
| 73 | + | } | |||
| 74 | + | } | |||
| 75 | + | ||||
| 64 | return m | 76 | return m | ||
| 65 | } | 77 | } | ||
| 66 | 78 | ||||
| 67 | // events returns the mergeability events published so far. | 79 | // events returns the mergeability events published so far. | ||
| 68 | func (r *mergeRig) events() []domain.MergeRequestMergeability { | 80 | func (r *mergeRig) events() []domain.MergeRequestMergeability { | ||
| 69 | - | var out []domain.MergeRequestMergeability | 81 | + | out := r.history |
| 82 | + | r.history = nil | |||
| 70 | for { | 83 | for { | ||
| 71 | select { | 84 | select { | ||
| 72 | case ev := <-r.mev: | 85 | case ev := <-r.mev: | ||
| @@ -83,7 +96,8 @@ func (r *mergeRig) events() []domain.MergeRequestMergeability { | |||||
| 83 | // bound: the workers publish after they stored the record the tests wait for. | 96 | // bound: the workers publish after they stored the record the tests wait for. | ||
| 84 | func (r *mergeRig) awaitEvents(n int) []domain.MergeRequestMergeability { | 97 | func (r *mergeRig) awaitEvents(n int) []domain.MergeRequestMergeability { | ||
| 85 | r.t.Helper() | 98 | r.t.Helper() | ||
| 86 | - | var out []domain.MergeRequestMergeability | 99 | + | out := r.history |
| 100 | + | r.history = nil | |||
| 87 | deadline := time.After(10 * time.Second) | 101 | deadline := time.After(10 * time.Second) | ||
| 88 | for len(out) < n { | 102 | for len(out) < n { | ||
| 89 | select { | 103 | select { | ||
| @@ -153,49 +167,29 @@ func TestMergeabilityReaderIsLazy(t *testing.T) { | |||||
| 153 | assert.Equal(t, domain.MergeabilityUpToDate, page.Mergeability[mr.Number].State) | 167 | assert.Equal(t, domain.MergeabilityUpToDate, page.Mergeability[mr.Number].State) | ||
| 154 | } | 168 | } | ||
| 155 | 169 | ||||
| 156 | - | func TestMergeabilityTransitionOnlyPublication(t *testing.T) { | 170 | + | func TestMergeabilityPublishesEachSettledResultOnce(t *testing.T) { |
| 157 | r := newMergeRig(t) | 171 | r := newMergeRig(t) | ||
| 158 | r.seed() | 172 | r.seed() | ||
| 159 | r.feature("feat") | 173 | r.feature("feat") | ||
| 160 | mr := r.open("feat") | 174 | mr := r.open("feat") | ||
| 161 | - | r.svc.Check(r.repo, mr.Number) | 175 | + | r.svc.ReconcileForTest(r.repo) |
| 162 | require.Len(t, r.events(), 1) | 176 | require.Len(t, r.events(), 1) | ||
| 163 | 177 | ||||
| 164 | // Same tips: nothing is computed, nothing is published. | 178 | // Same tips: nothing is computed, nothing is published. | ||
| 165 | - | r.svc.Check(r.repo, mr.Number) | 179 | + | r.svc.ReconcileForTest(r.repo) |
| 166 | assert.Empty(t, r.events()) | 180 | assert.Empty(t, r.events()) | ||
| 167 | 181 | ||||
| 168 | - | // The target moves without changing the state: a new record but, being the | 182 | + | // New tips publish even when the state and counts stay the same. |
| 169 | - | // same state and counts, no event. | |||
| 170 | r.sh("checkout", "-q", "feat") | 183 | r.sh("checkout", "-q", "feat") | ||
| 171 | r.commit(author, "e.txt", "e\n", "more") | 184 | r.commit(author, "e.txt", "e\n", "more") | ||
| 172 | r.push("feat") | 185 | r.push("feat") | ||
| 173 | - | r.svc.Check(r.repo, mr.Number) | 186 | + | r.svc.ReconcileForTest(r.repo) |
| 174 | - | assert.Empty(t, r.events()) | 187 | + | assert.Len(t, r.events(), 1) |
| 175 | m, err := r.record(mr.Number) | 188 | m, err := r.record(mr.Number) | ||
| 176 | require.NoError(t, err) | 189 | require.NoError(t, err) | ||
| 177 | assert.Equal(t, domain.MergeabilityUpToDate, m.State) | 190 | assert.Equal(t, domain.MergeabilityUpToDate, m.State) | ||
| 178 | } | 191 | } | ||
| 179 | 192 | ||||
| 180 | - | func TestMergeabilityPublishesWhenReadersSawChecking(t *testing.T) { | |||
| 181 | - | r := newMergeRig(t) | |||
| 182 | - | r.seed() | |||
| 183 | - | r.feature("feat") | |||
| 184 | - | mr := r.open("feat") | |||
| 185 | - | r.svc.Check(r.repo, mr.Number) | |||
| 186 | - | require.Len(t, r.events(), 1) | |||
| 187 | - | ||||
| 188 | - | r.sh("checkout", "-q", "feat") | |||
| 189 | - | r.commit(author, "e.txt", "e\n", "more") | |||
| 190 | - | r.push("feat") | |||
| 191 | - | _, err := r.mrs.Get(r.ctx, root, r.repo, mr.Number, ports.GetMergeRequestOptions{}) // sees "checking" | |||
| 192 | - | require.NoError(t, err) | |||
| 193 | - | r.svc.Check(r.repo, mr.Number) | |||
| 194 | - | got := r.events() | |||
| 195 | - | require.Len(t, got, 1) | |||
| 196 | - | assert.Equal(t, domain.MergeabilityUpToDate, got[0].Mergeability.State) | |||
| 197 | - | } | |||
| 198 | - | ||||
| 199 | func TestMergeabilityMergeReevaluatesOtherRequests(t *testing.T) { | 193 | func TestMergeabilityMergeReevaluatesOtherRequests(t *testing.T) { | ||
| 200 | r := newMergeRig(t) | 194 | r := newMergeRig(t) | ||
| 201 | r.svc.Start() | 195 | r.svc.Start() | ||
| @@ -223,7 +217,7 @@ func TestMergeabilityClosedRequestRecordDeleted(t *testing.T) { | |||||
| 223 | r.seed() | 217 | r.seed() | ||
| 224 | r.feature("feat") | 218 | r.feature("feat") | ||
| 225 | mr := r.open("feat") | 219 | mr := r.open("feat") | ||
| 226 | - | r.svc.Check(r.repo, mr.Number) | 220 | + | r.svc.ReconcileForTest(r.repo) |
| 227 | _, err := r.record(mr.Number) | 221 | _, err := r.record(mr.Number) | ||
| 228 | require.NoError(t, err) | 222 | require.NoError(t, err) | ||
| 229 | 223 | ||||
| @@ -231,7 +225,7 @@ func TestMergeabilityClosedRequestRecordDeleted(t *testing.T) { | |||||
| 231 | require.NoError(t, err) | 225 | require.NoError(t, err) | ||
| 232 | _, err = r.record(mr.Number) | 226 | _, err = r.record(mr.Number) | ||
| 233 | assert.ErrorIs(t, err, domain.ErrNotFound) | 227 | assert.ErrorIs(t, err, domain.ErrNotFound) | ||
| 234 | - | r.svc.Check(r.repo, mr.Number) // a late check writes nothing | 228 | + | r.svc.ReconcileForTest(r.repo) // a late check writes nothing |
| 235 | _, err = r.record(mr.Number) | 229 | _, err = r.record(mr.Number) | ||
| 236 | assert.ErrorIs(t, err, domain.ErrNotFound) | 230 | assert.ErrorIs(t, err, domain.ErrNotFound) | ||
| 237 | } | 231 | } | ||
| @@ -244,7 +238,12 @@ func TestMergeabilityStartupRescanComputesMissingRecords(t *testing.T) { | |||||
| 244 | _, err := r.record(mr.Number) | 238 | _, err := r.record(mr.Number) | ||
| 245 | require.ErrorIs(t, err, domain.ErrNotFound) | 239 | require.ErrorIs(t, err, domain.ErrNotFound) | ||
| 246 | 240 | ||||
| 247 | - | r.svc.Start() | 241 | + | // A fresh service has received no OnOpened or Read trigger. |
| 242 | + | svc := usecase.NewMergeability(r.ctx, usecase.MergeabilityConfig{ | |||
| 243 | + | Store: r.store.MergeRequests(), Meta: r.store, Git: r.git, Bus: r.bus, Clock: fixedClock{}, | |||
| 244 | + | }) | |||
| 245 | + | defer svc.Close(context.Background()) | |||
| 246 | + | svc.Start() | |||
| 248 | r.waitState(mr.Number, domain.MergeabilityUpToDate, main, head) | 247 | r.waitState(mr.Number, domain.MergeabilityUpToDate, main, head) | ||
| 249 | } | 248 | } | ||
| 250 | 249 | ||||
| @@ -284,7 +283,7 @@ func newMockRig(t *testing.T) *mockRig { | |||||
| 284 | store: mocks.NewMergeRequestStore(t), meta: mocks.NewRepoMetaStore(t), git: mocks.NewMergeGit(t), bus: mocks.NewEventBus(t), | 283 | store: mocks.NewMergeRequestStore(t), meta: mocks.NewRepoMetaStore(t), git: mocks.NewMergeGit(t), bus: mocks.NewEventBus(t), | ||
| 285 | mr: domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen}, | 284 | mr: domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen}, | ||
| 286 | } | 285 | } | ||
| 287 | - | r.store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(r.mr, nil).Maybe() | 286 | + | r.store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{r.mr}, nil).Maybe() |
| 288 | r.store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound).Maybe() | 287 | r.store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound).Maybe() | ||
| 289 | r.meta.EXPECT().List(mock.Anything).Return(nil, nil).Maybe() | 288 | r.meta.EXPECT().List(mock.Anything).Return(nil, nil).Maybe() | ||
| 290 | r.meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil).Maybe() | 289 | r.meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil).Maybe() | ||
| @@ -295,45 +294,18 @@ func newMockRig(t *testing.T) *mockRig { | |||||
| 295 | return r | 294 | return r | ||
| 296 | } | 295 | } | ||
| 297 | 296 | ||||
| 298 | - | func TestMergeabilityStaleResultIsDiscarded(t *testing.T) { | |||
| 299 | - | r := newMockRig(t) | |||
| 300 | - | repo := r.mr.Repo | |||
| 301 | - | head, t1, t2 := fakeOID(t, "a"), fakeOID(t, "b"), fakeOID(t, "c") | |||
| 302 | - | var moved atomic.Bool | |||
| 303 | - | r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |||
| 304 | - | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").RunAndReturn( | |||
| 305 | - | func(context.Context, domain.RepoName, string) (domain.ObjectID, error) { | |||
| 306 | - | if moved.Load() { | |||
| 307 | - | return t2, nil | |||
| 308 | - | } | |||
| 309 | - | return t1, nil | |||
| 310 | - | }) | |||
| 311 | - | // The target moves while the first computation runs. | |||
| 312 | - | r.git.EXPECT().IsAncestor(mock.Anything, repo, mock.Anything, head).RunAndReturn( | |||
| 313 | - | func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID) (bool, error) { | |||
| 314 | - | moved.Store(true) | |||
| 315 | - | return true, nil | |||
| 316 | - | }) | |||
| 317 | - | var puts []domain.Mergeability | |||
| 318 | - | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn( | |||
| 319 | - | func(_ context.Context, m domain.Mergeability) error { puts = append(puts, m); return nil }) | |||
| 320 | - | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | |||
| 321 | - | ||||
| 322 | - | r.svc.Check(repo, 7) | |||
| 323 | - | require.Len(t, puts, 1, "the result for the old tip is dropped, the one for the new tip is stored") | |||
| 324 | - | assert.Equal(t, t2, puts[0].TargetID) | |||
| 325 | - | } | |||
| 326 | - | ||||
| 327 | func TestMergeabilityDedupsTriggers(t *testing.T) { | 297 | func TestMergeabilityDedupsTriggers(t *testing.T) { | ||
| 328 | r := newMockRig(t) | 298 | r := newMockRig(t) | ||
| 329 | repo := r.mr.Repo | 299 | repo := r.mr.Repo | ||
| 330 | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | 300 | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | ||
| 331 | r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | 301 | r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | ||
| 332 | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | 302 | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | ||
| 303 | + | computed := make(chan struct{}, 1) | |||
| 333 | var computes atomic.Int32 | 304 | var computes atomic.Int32 | ||
| 334 | r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).RunAndReturn( | 305 | r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).RunAndReturn( | ||
| 335 | func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID) (bool, error) { | 306 | func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID) (bool, error) { | ||
| 336 | computes.Add(1) | 307 | computes.Add(1) | ||
| 308 | + | computed <- struct{}{} | |||
| 337 | return true, nil | 309 | return true, nil | ||
| 338 | }) | 310 | }) | ||
| 339 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | 311 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | ||
| @@ -343,7 +315,7 @@ func TestMergeabilityDedupsTriggers(t *testing.T) { | |||||
| 343 | r.svc.OnOpened(context.Background(), r.mr) // queued before any worker runs | 315 | r.svc.OnOpened(context.Background(), r.mr) // queued before any worker runs | ||
| 344 | } | 316 | } | ||
| 345 | r.svc.Start() | 317 | r.svc.Start() | ||
| 346 | - | require.Eventually(t, func() bool { return computes.Load() >= 1 }, 5*time.Second, 5*time.Millisecond) | 318 | + | <-computed |
| 347 | r.svc.Close(context.Background()) // the workers have each looked at the queue once more | 319 | r.svc.Close(context.Background()) // the workers have each looked at the queue once more | ||
| 348 | assert.EqualValues(t, 1, computes.Load()) | 320 | assert.EqualValues(t, 1, computes.Load()) | ||
| 349 | } | 321 | } | ||
| @@ -356,71 +328,7 @@ func TestMergeabilityNoWriteForClosedRequest(t *testing.T) { | |||||
| 356 | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | 328 | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | ||
| 357 | r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(true, nil) | 329 | r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(true, nil) | ||
| 358 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(domain.ErrNotFound).Once() // closed meanwhile | 330 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(domain.ErrNotFound).Once() // closed meanwhile | ||
| 359 | - | r.svc.Check(repo, 7) // no event, no further write | 331 | + | r.svc.ReconcileForTest(repo) // no event, no further write |
| 360 | - | } | |||
| 361 | - | ||||
| 362 | - | // A reader that saw a stale record marks the request seen and queues it; the | |||
| 363 | - | // worker finds the record already current (stored and published by an earlier | |||
| 364 | - | // check) and must still publish it, as the reader was told "checking". | |||
| 365 | - | func TestMergeabilityFastPathPublishesWhenAReaderSawChecking(t *testing.T) { | |||
| 366 | - | repo := name(t, "proj") | |||
| 367 | - | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |||
| 368 | - | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen, HeadID: head} | |||
| 369 | - | rec := domain.Mergeability{Repo: repo, Number: 7, State: domain.MergeabilityUpToDate, TargetID: tip, HeadID: head, CheckedAt: now} | |||
| 370 | - | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | |||
| 371 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil) | |||
| 372 | - | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound).Once() // the reader's read: stale | |||
| 373 | - | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(rec, nil) // the worker has stored it since | |||
| 374 | - | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil) | |||
| 375 | - | git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |||
| 376 | - | git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |||
| 377 | - | var published []domain.Event | |||
| 378 | - | bus.EXPECT().Publish(mock.Anything, mock.Anything).Run(func(_ context.Context, ev domain.Event) { published = append(published, ev) }).Return().Once() | |||
| 379 | - | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}}) | |||
| 380 | - | t.Cleanup(func() { svc.Close(context.Background()) }) | |||
| 381 | - | ||||
| 382 | - | got := svc.Read(context.Background(), mr, tip) | |||
| 383 | - | assert.Equal(t, domain.MergeabilityChecking, got.State) | |||
| 384 | - | svc.Check(repo, 7) | |||
| 385 | - | require.Len(t, published, 1, "the current record is published for the reader") | |||
| 386 | - | assert.Equal(t, domain.MergeabilityUpToDate, published[0].(domain.MergeRequestMergeability).Mergeability.State) | |||
| 387 | - | ||||
| 388 | - | svc.Check(repo, 7) // nobody saw "checking" since: nothing more | |||
| 389 | - | assert.Len(t, published, 1) | |||
| 390 | - | } | |||
| 391 | - | ||||
| 392 | - | // A rescan claims a key that is also pending; a worker must then not run it | |||
| 393 | - | // concurrently (GO-001). | |||
| 394 | - | func TestMergeabilityRescanAndWorkersNeverRunAKeyConcurrently(t *testing.T) { | |||
| 395 | - | r := newMockRig(t) | |||
| 396 | - | repo := r.mr.Repo | |||
| 397 | - | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |||
| 398 | - | r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |||
| 399 | - | r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |||
| 400 | - | var inflight, peak atomic.Int32 | |||
| 401 | - | entered, release := make(chan struct{}, 8), make(chan struct{}) | |||
| 402 | - | r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).RunAndReturn( | |||
| 403 | - | func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID) (bool, error) { | |||
| 404 | - | n := inflight.Add(1) | |||
| 405 | - | for p := peak.Load(); n > p && !peak.CompareAndSwap(p, n); p = peak.Load() { | |||
| 406 | - | } | |||
| 407 | - | entered <- struct{}{} | |||
| 408 | - | <-release | |||
| 409 | - | inflight.Add(-1) | |||
| 410 | - | return true, nil | |||
| 411 | - | }) | |||
| 412 | - | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | |||
| 413 | - | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | |||
| 414 | - | ||||
| 415 | - | r.svc.OnOpened(context.Background(), r.mr) // pending | |||
| 416 | - | done := make(chan struct{}) | |||
| 417 | - | go func() { r.svc.Check(repo, 7); close(done) }() // claims it, as a rescan does | |||
| 418 | - | <-entered | |||
| 419 | - | r.svc.Start() // two workers | |||
| 420 | - | r.svc.Close(context.Background()) | |||
| 421 | - | close(release) | |||
| 422 | - | <-done | |||
| 423 | - | assert.EqualValues(t, 1, peak.Load(), "one check of the key at a time") | |||
| 424 | } | 332 | } | ||
| 425 | 333 | ||||
| 426 | func TestMergeabilityBoundsConflictPathsAndBehind(t *testing.T) { | 334 | func TestMergeabilityBoundsConflictPathsAndBehind(t *testing.T) { | ||
| @@ -440,7 +348,7 @@ func TestMergeabilityBoundsConflictPathsAndBehind(t *testing.T) { | |||||
| 440 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn( | 348 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn( | ||
| 441 | func(_ context.Context, m domain.Mergeability) error { put = m; return nil }) | 349 | func(_ context.Context, m domain.Mergeability) error { put = m; return nil }) | ||
| 442 | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | 350 | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | ||
| 443 | - | r.svc.Check(repo, 7) | 351 | + | r.svc.ReconcileForTest(repo) |
| 444 | assert.Equal(t, domain.MergeabilityConflict, put.State) | 352 | assert.Equal(t, domain.MergeabilityConflict, put.State) | ||
| 445 | assert.Len(t, put.ConflictPaths, domain.MaxConflictPaths) | 353 | assert.Len(t, put.ConflictPaths, domain.MaxConflictPaths) | ||
| 446 | assert.True(t, put.ConflictPathsTruncated) | 354 | assert.True(t, put.ConflictPathsTruncated) | ||
| @@ -458,7 +366,7 @@ func TestMergeabilityBoundsConflictPathsAndBehind(t *testing.T) { | |||||
| 458 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn( | 366 | r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn( | ||
| 459 | func(_ context.Context, m domain.Mergeability) error { put = m; return nil }) | 367 | func(_ context.Context, m domain.Mergeability) error { put = m; return nil }) | ||
| 460 | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | 368 | r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return() | ||
| 461 | - | r.svc.Check(repo, 7) | 369 | + | r.svc.ReconcileForTest(repo) |
| 462 | assert.Equal(t, domain.MaxBehindCount, put.Behind) | 370 | assert.Equal(t, domain.MaxBehindCount, put.Behind) | ||
| 463 | assert.True(t, put.BehindCapped) | 371 | assert.True(t, put.BehindCapped) | ||
| 464 | }) | 372 | }) | ||
| @@ -471,10 +379,9 @@ func TestMergeabilityRetriesATransientFailureWithBackoff(t *testing.T) { | |||||
| 471 | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | 379 | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | ||
| 472 | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | 380 | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | ||
| 473 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | 381 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | ||
| 474 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil) | 382 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil) |
| 475 | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | 383 | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | ||
| 476 | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | 384 | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | ||
| 477 | - | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil).Once() | |||
| 478 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | 385 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | ||
| 479 | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil) | 386 | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil) | ||
| 480 | var fails atomic.Int32 | 387 | var fails atomic.Int32 | ||
| @@ -515,7 +422,8 @@ func TestMergeabilityCloseStopsPendingRetries(t *testing.T) { | |||||
| 515 | repo := name(t, "proj") | 422 | repo := name(t, "proj") | ||
| 516 | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | 423 | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | ||
| 517 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | 424 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | ||
| 518 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(domain.MergeRequest{}, errors.New("db down")) | 425 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return(nil, errors.New("db down")) |
| 426 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo}, nil) | |||
| 519 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | 427 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | ||
| 520 | asked := make(chan time.Duration, 4) | 428 | asked := make(chan time.Duration, 4) | ||
| 521 | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | 429 | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | ||
| @@ -537,7 +445,7 @@ func TestMergeabilityWorkersOutliveTheParentContext(t *testing.T) { | |||||
| 537 | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | 445 | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | ||
| 538 | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | 446 | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | ||
| 539 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | 447 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | ||
| 540 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil) | 448 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil) |
| 541 | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | 449 | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | ||
| 542 | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | 450 | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil) | ||
| 543 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | 451 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | ||
| @@ -560,27 +468,33 @@ func TestMergeabilityWorkersOutliveTheParentContext(t *testing.T) { | |||||
| 560 | // Failures of many requests of 3 repositories schedule at most one retry per | 468 | // Failures of many requests of 3 repositories schedule at most one retry per | ||
| 561 | // repository on a single scheduler goroutine. | 469 | // repository on a single scheduler goroutine. | ||
| 562 | func TestMergeabilityRetriesAreBounded(t *testing.T) { | 470 | func TestMergeabilityRetriesAreBounded(t *testing.T) { | ||
| 563 | - | const repos, perRepo = 3, 20 | 471 | + | const repos = 3 |
| 564 | - | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | 472 | + | store, meta := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t) |
| 565 | - | store.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(domain.MergeRequest{}, errors.New("db down")) | 473 | + | barrier := name(t, "barrier") |
| 474 | + | done := make(chan struct{}) | |||
| 475 | + | store.EXPECT().ListOpen(mock.Anything, barrier).RunAndReturn(func(context.Context, domain.RepoName) ([]domain.MergeRequest, error) { close(done); return nil, nil }).Once() | |||
| 476 | + | store.EXPECT().ListOpen(mock.Anything, mock.Anything).Return(nil, errors.New("db down")) | |||
| 477 | + | meta.EXPECT().Get(mock.Anything, mock.Anything).Return(domain.Repo{}, nil) | |||
| 566 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | 478 | meta.EXPECT().List(mock.Anything).Return(nil, nil) | ||
| 479 | + | asked := make(chan time.Duration, repos) | |||
| 567 | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | 480 | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | ||
| 568 | - | Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}, Workers: 1, Queue: 4, | 481 | + | Store: store, Meta: meta, Clock: fixedClock{}, Workers: 1, |
| 569 | - | After: func(time.Duration) <-chan time.Time { return make(chan time.Time) }, | 482 | + | After: func(d time.Duration) <-chan time.Time { asked <- d; return make(chan time.Time) }, |
| 570 | }) | 483 | }) | ||
| 571 | svc.Start() | 484 | svc.Start() | ||
| 572 | - | before := runtime.NumGoroutine() | |||
| 573 | for r := range repos { | 485 | for r := range repos { | ||
| 574 | repo := name(t, "proj"+strconv.Itoa(r)) | 486 | repo := name(t, "proj"+strconv.Itoa(r)) | ||
| 575 | - | for n := range perRepo { | 487 | + | for range 20 { |
| 576 | - | svc.Check(repo, int64(n+1)) | 488 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: repo}) |
| 577 | } | 489 | } | ||
| 578 | } | 490 | } | ||
| 579 | - | assert.Positive(t, svc.ScheduledRetries()) | 491 | + | svc.OnOpened(context.Background(), domain.MergeRequest{Repo: barrier}) |
| 580 | - | assert.LessOrEqual(t, svc.ScheduledRetries(), repos+1) | 492 | + | <-done |
| 581 | - | assert.LessOrEqual(t, svc.RetryAttempts(), repos+1) | 493 | + | // The shared scheduler only arms the earliest pending retry. |
| 582 | - | assert.LessOrEqual(t, runtime.NumGoroutine(), before, "retries must not start goroutines") | 494 | + | <-asked |
| 583 | - | svc.Close(context.Background()) // returns although retries are pending | 495 | + | svc.Close(context.Background()) |
| 496 | + | assert.Equal(t, repos, svc.ScheduledRetries()) | |||
| 497 | + | assert.Equal(t, repos, svc.RetryAttempts()) | |||
| 584 | } | 498 | } | ||
| 585 | 499 | ||||
| 586 | // retryRig is a service over mocks whose only repository holds request !7, | 500 | // retryRig is a service over mocks whose only repository holds request !7, | ||
| @@ -592,26 +506,28 @@ type retryRig struct { | |||||
| 592 | failFor atomic.Int32 | 506 | failFor atomic.Int32 | ||
| 593 | passes chan struct{} | 507 | passes chan struct{} | ||
| 594 | afters chan chan time.Time | 508 | afters chan chan time.Time | ||
| 509 | + | barrier chan struct{} | |||
| 595 | } | 510 | } | ||
| 596 | 511 | ||||
| 597 | func newRetryRig(t *testing.T) *retryRig { | 512 | func newRetryRig(t *testing.T) *retryRig { | ||
| 598 | t.Helper() | 513 | t.Helper() | ||
| 599 | - | r := &retryRig{repo: name(t, "proj"), passes: make(chan struct{}, 32), afters: make(chan chan time.Time, 32)} | 514 | + | r := &retryRig{repo: name(t, "proj"), passes: make(chan struct{}, 32), afters: make(chan chan time.Time, 32), barrier: make(chan struct{})} |
| 600 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | 515 | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | ||
| 601 | r.failFor.Store(1 << 30) | 516 | r.failFor.Store(1 << 30) | ||
| 602 | - | store.EXPECT().Get(mock.Anything, r.repo, int64(7)).RunAndReturn( | 517 | + | meta.EXPECT().Get(mock.Anything, r.repo).Return(domain.Repo{Name: r.repo}, nil) |
| 603 | - | func(context.Context, domain.RepoName, int64) (domain.MergeRequest, error) { | |||
| 604 | - | if r.reads.Add(1) <= r.failFor.Load() { | |||
| 605 | - | return domain.MergeRequest{}, errors.New("db down") | |||
| 606 | - | } | |||
| 607 | - | return domain.MergeRequest{}, domain.ErrNotFound | |||
| 608 | - | }) | |||
| 609 | store.EXPECT().ListOpen(mock.Anything, r.repo).RunAndReturn( | 518 | store.EXPECT().ListOpen(mock.Anything, r.repo).RunAndReturn( | ||
| 610 | func(context.Context, domain.RepoName) ([]domain.MergeRequest, error) { | 519 | func(context.Context, domain.RepoName) ([]domain.MergeRequest, error) { | ||
| 611 | - | r.passes <- struct{}{} | 520 | + | defer func() { r.passes <- struct{}{} }() |
| 612 | - | return []domain.MergeRequest{{Repo: r.repo, Number: 7}}, nil | 521 | + | if r.reads.Add(1) <= r.failFor.Load() { |
| 522 | + | return nil, errors.New("db down") | |||
| 523 | + | } | |||
| 524 | + | return nil, nil | |||
| 613 | }) | 525 | }) | ||
| 614 | - | store.EXPECT().CountOpen(mock.Anything, r.repo).Return(1, nil) | 526 | + | barrier := name(t, "barrier") |
| 527 | + | meta.EXPECT().Get(mock.Anything, barrier).RunAndReturn(func(context.Context, domain.RepoName) (domain.Repo, error) { | |||
| 528 | + | r.barrier <- struct{}{} | |||
| 529 | + | return domain.Repo{}, domain.ErrNotFound | |||
| 530 | + | }) | |||
| 615 | meta.EXPECT().List(mock.Anything).Return([]domain.Repo{{Name: r.repo}}, nil) | 531 | meta.EXPECT().List(mock.Anything).Return([]domain.Repo{{Name: r.repo}}, nil) | ||
| 616 | r.svc = usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | 532 | r.svc = usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | ||
| 617 | Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}, Workers: 1, | 533 | Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}, Workers: 1, | ||
| @@ -630,8 +546,10 @@ func (r *retryRig) next() { | |||||
| 630 | 546 | ||||
| 631 | func (r *retryRig) settled(t *testing.T) { | 547 | func (r *retryRig) settled(t *testing.T) { | ||
| 632 | t.Helper() | 548 | t.Helper() | ||
| 633 | - | require.Eventually(t, func() bool { return r.svc.RetryAttempts() == 0 && r.svc.ScheduledRetries() == 0 }, | 549 | + | r.svc.OnOpened(context.Background(), domain.MergeRequest{Repo: name(t, "barrier")}) |
| 634 | - | 10*time.Second, time.Millisecond) | 550 | + | <-r.barrier |
| 551 | + | require.Zero(t, r.svc.RetryAttempts()) | |||
| 552 | + | require.Zero(t, r.svc.ScheduledRetries()) | |||
| 635 | } | 553 | } | ||
| 636 | 554 | ||||
| 637 | // A persistent failure is retried 5 times, then given up with no state kept. | 555 | // A persistent failure is retried 5 times, then given up with no state kept. | ||
| @@ -648,7 +566,7 @@ func TestMergeabilityGivesUpAfterFiveRetries(t *testing.T) { | |||||
| 648 | } | 566 | } | ||
| 649 | 567 | ||||
| 650 | // A successful full rescan resets the budget: a later failure gets 5 again. | 568 | // A successful full rescan resets the budget: a later failure gets 5 again. | ||
| 651 | - | func TestMergeabilityFullRescanResetsTheRetryBudget(t *testing.T) { | 569 | + | func TestMergeabilitySuccessfulPassForgetsTheRetryBudget(t *testing.T) { |
| 652 | r := newRetryRig(t) | 570 | r := newRetryRig(t) | ||
| 653 | r.failFor.Store(2) // the first two reads fail, the third succeeds | 571 | r.failFor.Store(2) // the first two reads fail, the third succeeds | ||
| 654 | r.svc.Start() | 572 | r.svc.Start() | ||
| @@ -657,7 +575,8 @@ func TestMergeabilityFullRescanResetsTheRetryBudget(t *testing.T) { | |||||
| 657 | r.next() // success | 575 | r.next() // success | ||
| 658 | r.settled(t) | 576 | r.settled(t) | ||
| 659 | r.failFor.Store(1 << 30) | 577 | r.failFor.Store(1 << 30) | ||
| 660 | - | r.svc.Check(r.repo, 7) // fails alone: retries the repository | 578 | + | r.svc.OnOpened(context.Background(), domain.MergeRequest{Repo: r.repo}) |
| 579 | + | <-r.passes | |||
| 661 | for range 5 { | 580 | for range 5 { | ||
| 662 | r.next() | 581 | r.next() | ||
| 663 | } | 582 | } | ||
| @@ -665,79 +584,3 @@ func TestMergeabilityFullRescanResetsTheRetryBudget(t *testing.T) { | |||||
| 665 | assert.Empty(t, r.afters) | 584 | assert.Empty(t, r.afters) | ||
| 666 | assert.Empty(t, r.passes) | 585 | assert.Empty(t, r.passes) | ||
| 667 | } | 586 | } | ||
| 668 | - | ||||
| 669 | - | // A transient failure to publish after the store is retried and the event is | |||
| 670 | - | // published without any other trigger. | |||
| 671 | - | func TestMergeabilityRetriesAFailedPublish(t *testing.T) { | |||
| 672 | - | repo := name(t, "proj") | |||
| 673 | - | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |||
| 674 | - | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | |||
| 675 | - | rec := domain.Mergeability{Repo: repo, Number: 7, TargetID: tip, HeadID: head, State: domain.MergeabilityUpToDate} | |||
| 676 | - | store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t) | |||
| 677 | - | store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil) | |||
| 678 | - | var gets atomic.Int32 | |||
| 679 | - | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).RunAndReturn( | |||
| 680 | - | func(context.Context, domain.RepoName, int64) (domain.Mergeability, error) { | |||
| 681 | - | if gets.Add(1) == 1 { | |||
| 682 | - | return domain.Mergeability{}, domain.ErrNotFound | |||
| 683 | - | } | |||
| 684 | - | return rec, nil | |||
| 685 | - | }) | |||
| 686 | - | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil).Once() | |||
| 687 | - | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil) | |||
| 688 | - | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |||
| 689 | - | var metaGets atomic.Int32 | |||
| 690 | - | meta.EXPECT().Get(mock.Anything, repo).RunAndReturn( | |||
| 691 | - | func(context.Context, domain.RepoName) (domain.Repo, error) { | |||
| 692 | - | if metaGets.Add(1) == 1 { | |||
| 693 | - | return domain.Repo{}, errors.New("transient") | |||
| 694 | - | } | |||
| 695 | - | return domain.Repo{Name: repo, CreatedAt: now}, nil | |||
| 696 | - | }) | |||
| 697 | - | git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |||
| 698 | - | git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |||
| 699 | - | git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(true, nil) | |||
| 700 | - | published := make(chan struct{}) | |||
| 701 | - | bus.EXPECT().Publish(mock.Anything, mock.Anything).Run(func(context.Context, domain.Event) { close(published) }).Return().Once() | |||
| 702 | - | ||||
| 703 | - | afters := make(chan chan time.Time, 4) | |||
| 704 | - | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |||
| 705 | - | Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}, | |||
| 706 | - | After: func(time.Duration) <-chan time.Time { ch := make(chan time.Time, 1); afters <- ch; return ch }, | |||
| 707 | - | }) | |||
| 708 | - | t.Cleanup(func() { svc.Close(context.Background()) }) | |||
| 709 | - | svc.Start() | |||
| 710 | - | svc.OnOpened(context.Background(), mr) | |||
| 711 | - | <-svcWaiting(afters) | |||
| 712 | - | select { | |||
| 713 | - | case <-published: | |||
| 714 | - | t.Fatal("published although the repository could not be read") | |||
| 715 | - | default: | |||
| 716 | - | } | |||
| 717 | - | stop := fireAll(afters) | |||
| 718 | - | defer stop() | |||
| 719 | - | <-published | |||
| 720 | - | } | |||
| 721 | - | ||||
| 722 | - | // svcWaiting blocks until the scheduler asked for a timer, and puts it back. | |||
| 723 | - | func svcWaiting(afters chan chan time.Time) <-chan struct{} { | |||
| 724 | - | ready := make(chan struct{}) | |||
| 725 | - | go func() { ch := <-afters; afters <- ch; close(ready) }() | |||
| 726 | - | return ready | |||
| 727 | - | } | |||
| 728 | - | ||||
| 729 | - | // fireAll fires every timer the scheduler asks for (stale ones are harmless). | |||
| 730 | - | func fireAll(afters chan chan time.Time) (stop func()) { | |||
| 731 | - | quit := make(chan struct{}) | |||
| 732 | - | go func() { | |||
| 733 | - | for { | |||
| 734 | - | select { | |||
| 735 | - | case ch := <-afters: | |||
| 736 | - | ch <- time.Time{} | |||
| 737 | - | case <-quit: | |||
| 738 | - | return | |||
| 739 | - | } | |||
| 740 | - | } | |||
| 741 | - | }() | |||
| 742 | - | return func() { close(quit) } | |||
| 743 | - | } | |||
M internal/usecase/mergerequests.go +0 -4
| @@ -873,9 +873,6 @@ func (m *MergeRequests) Close(ctx context.Context, p domain.Principal, name doma | |||
| 873 | 873 | Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("merge request closed") | |
| 874 | 874 | recordActivity(ctx, m.meta, repo.Name, mr.UpdatedAt) | |
| 875 | 875 | m.bus.Publish(ctx, domain.MergeRequestClosed{MergeRequest: mr, At: mr.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | |
| 876 | - | if m.mergeab != nil { | |
| 877 | - | m.mergeab.OnClosed(ctx, mr) | |
| 878 | - | } | |
| 879 | 876 | if m.pulls != nil { | |
| 880 | 877 | m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID}, p) | |
| 881 | 878 | } | |
| @@ -1156,7 +1153,6 @@ func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo | |||
| 1156 | 1153 | m.ci.OnMerge(ctx, name, updates, p) | |
| 1157 | 1154 | } | |
| 1158 | 1155 | if m.mergeab != nil { | |
| 1159 | - | m.mergeab.OnClosed(ctx, mr) | |
| 1160 | 1156 | m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target | |
| 1161 | 1157 | } | |
| 1162 | 1158 | } | |
| @@ -873,9 +873,6 @@ func (m *MergeRequests) Close(ctx context.Context, p domain.Principal, name doma | |||||
| 873 | Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("merge request closed") | 873 | Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("merge request closed") | ||
| 874 | recordActivity(ctx, m.meta, repo.Name, mr.UpdatedAt) | 874 | recordActivity(ctx, m.meta, repo.Name, mr.UpdatedAt) | ||
| 875 | m.bus.Publish(ctx, domain.MergeRequestClosed{MergeRequest: mr, At: mr.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | 875 | m.bus.Publish(ctx, domain.MergeRequestClosed{MergeRequest: mr, At: mr.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | ||
| 876 | - | if m.mergeab != nil { | |||
| 877 | - | m.mergeab.OnClosed(ctx, mr) | |||
| 878 | - | } | |||
| 879 | if m.pulls != nil { | 876 | if m.pulls != nil { | ||
| 880 | m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID}, p) | 877 | m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID}, p) | ||
| 881 | } | 878 | } | ||
| @@ -1156,7 +1153,6 @@ func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo | |||||
| 1156 | m.ci.OnMerge(ctx, name, updates, p) | 1153 | m.ci.OnMerge(ctx, name, updates, p) | ||
| 1157 | } | 1154 | } | ||
| 1158 | if m.mergeab != nil { | 1155 | if m.mergeab != nil { | ||
| 1159 | - | m.mergeab.OnClosed(ctx, mr) | |||
| 1160 | m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target | 1156 | m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target | ||
| 1161 | } | 1157 | } | ||
| 1162 | } | 1158 | } | ||