fix(mr): count retry attempts only for the keys that wait in the scheduler

commit: 99b00c335f6212a20fd9fc84655748ae0f9cf3f6
author: brice · 3d ago · 2026-10-19 15:18 UTC
committer: brice · 2d ago · 2026-10-19 18:30 UTC
parent: 33a90cd
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +18 -7

M internal/usecase/export_test.go +7 -0


@@ -182,3 +182,10 @@ func (s *Mergeability) ScheduledRetries() int {
182	182		defer s.retryMu.Unlock()
183	183		return len(s.heap)
184	184		}
	185	+
	186	+	// RetryAttempts gives the number of keys whose failed attempts are counted.
	187	+	func (s *Mergeability) RetryAttempts() int {
	188	+	s.retryMu.Lock()
	189	+	defer s.retryMu.Unlock()
	190	+	return len(s.retries)
	191	+	}


@@ -182,3 +182,10 @@ func (s *Mergeability) ScheduledRetries() int {
182	defer s.retryMu.Unlock()	182		defer s.retryMu.Unlock()
183	return len(s.heap)	183		return len(s.heap)
184	}	184		}
		185	+
		186	+	// RetryAttempts gives the number of keys whose failed attempts are counted.
		187	+	func (s *Mergeability) RetryAttempts() int {
		188	+	s.retryMu.Lock()
		189	+	defer s.retryMu.Unlock()
		190	+	return len(s.retries)
		191	+	}

M internal/usecase/mergeability.go +10 -7


@@ -161,19 +161,22 @@ func (s *Mergeability) retry(key retryKey) {
161	161		if s.closed \|\| s.scheduled[key] {
162	162		return
163	163		}
164		-	s.retries[key]++
165		-	n := s.retries[key]
166		-	if n > mergeabilityRetries {
167		-	delete(s.retries, key)
168		-	return
169		-	}
	164	+	// The key that waits is chosen before counting attempts, so that only
	165	+	// the waiting keys hold attempt counts: at most cfg.Queue requests plus
	166	+	// one per repository.
170	167		sk := key
171		-	if key.kind == retryCheck && s.checkRetries >= s.cfg.Queue {
	168	+	if key.kind == retryCheck && s.retries[key] == 0 && s.checkRetries >= s.cfg.Queue {
172	169		sk = retryKey{kind: retryRescan, mk: mergeKey{repo: key.mk.repo}}
173	170		if s.scheduled[sk] {
174	171		return
175	172		}
176	173		}
	174	+	s.retries[sk]++
	175	+	n := s.retries[sk]
	176	+	if n > mergeabilityRetries {
	177	+	delete(s.retries, sk)
	178	+	return
	179	+	}
177	180		if sk.kind == retryCheck {
178	181		s.checkRetries++
179	182		}

 	if s.closed || s.scheduled[key] {
 		return
 	}
-	s.retries[key]++
+	// The key that waits is chosen before counting attempts, so that only
-	n := s.retries[key]
+	// the waiting keys hold attempt counts: at most cfg.Queue requests plus
-	if n > mergeabilityRetries {
+	// one per repository.
-		delete(s.retries, key)
-		return
-	}
 	sk := key
-	if key.kind == retryCheck && s.checkRetries >= s.cfg.Queue {
+	if key.kind == retryCheck && s.retries[key] == 0 && s.checkRetries >= s.cfg.Queue {
 		sk = retryKey{kind: retryRescan, mk: mergeKey{repo: key.mk.repo}}
 		if s.scheduled[sk] {
 			return
 		}
 	}
+	s.retries[sk]++
+	n := s.retries[sk]
+	if n > mergeabilityRetries {
+		delete(s.retries, sk)
+		return
+	}
 	if sk.kind == retryCheck {
 		s.checkRetries++
 	}

M internal/usecase/mergeability_test.go +1 -0


@@ -576,6 +576,7 @@ func TestMergeabilityRetriesAreBounded(t *testing.T) {
576	576		}
577	577		assert.LessOrEqual(t, svc.ScheduledRetries(), queue+repos)
578	578		assert.Positive(t, svc.ScheduledRetries())
	579	+	assert.LessOrEqual(t, svc.RetryAttempts(), queue+repos, "only the waiting keys count their attempts")
579	580		assert.LessOrEqual(t, runtime.NumGoroutine(), before, "retries must not start goroutines")
580	581		svc.Close(context.Background()) // returns although retries are pending
581	582		}


@@ -576,6 +576,7 @@ func TestMergeabilityRetriesAreBounded(t *testing.T) {
576	}	576		}
577	assert.LessOrEqual(t, svc.ScheduledRetries(), queue+repos)	577		assert.LessOrEqual(t, svc.ScheduledRetries(), queue+repos)
578	assert.Positive(t, svc.ScheduledRetries())	578		assert.Positive(t, svc.ScheduledRetries())
		579	+	assert.LessOrEqual(t, svc.RetryAttempts(), queue+repos, "only the waiting keys count their attempts")
579	assert.LessOrEqual(t, runtime.NumGoroutine(), before, "retries must not start goroutines")	580		assert.LessOrEqual(t, runtime.NumGoroutine(), before, "retries must not start goroutines")
580	svc.Close(context.Background()) // returns although retries are pending	581		svc.Close(context.Background()) // returns although retries are pending
581	}	582		}