Explorar el Código

Retry fewest-attempted export windows first

With 12 or more windows that always fail, one of them was due on every
tick and, being oldest, took the single retry slot before fail-fast
stopped further retries, so a newer window that failed once could wait
until the failing windows were evicted. Due retries are now ordered by
fewest failed attempts, then oldest.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: peatey <warwick@automatic.systems>
peatey hace 23 horas
padre
commit
ecc8ebd0eb
Se han modificado 2 ficheros con 52 adiciones y 6 borrados
  1. 24 6
      core/pkg/exporter/controller.go
  2. 28 0
      core/pkg/exporter/controller_pending_test.go

+ 24 - 6
core/pkg/exporter/controller.go

@@ -219,7 +219,9 @@ func (cd *ComputeExportController[T]) Start(interval time.Duration) bool {
 }
 
 // tick runs a single export pass for the provided time: newly closed windows first, then due retries
-// oldest first (together at most maxExportsPerTick), followed by the current window.
+// (together at most maxExportsPerTick), followed by the current window. Retries go to the windows with
+// the fewest failed attempts first, then oldest, so windows that keep failing can't hold the retry
+// slot ahead of a window that failed once.
 func (cd *ComputeExportController[T]) tick(now time.Time) {
 	cd.tickCount++
 
@@ -230,15 +232,28 @@ func (cd *ComputeExportController[T]) tick(now time.Time) {
 		cd.lastTickWindow = start
 	}
 
+	// candidates for each pass: never-attempted windows oldest first, then due retries ordered by
+	// fewest attempts, then oldest (a stable sort keeps pending's ascending order within ties)
+	var first, retries []*pendingWindow
+	for _, pw := range cd.pending {
+		if pw.attempts == 0 {
+			first = append(first, pw)
+		} else if pw.nextTick <= cd.tickCount {
+			retries = append(retries, pw)
+		}
+	}
+	slices.SortStableFunc(retries, func(a, b *pendingWindow) int { return a.attempts - b.attempts })
+
 	attempts := 0
-	for _, firstAttempt := range []bool{true, false} {
-		for _, pw := range cd.pending {
+	for _, pass := range []struct {
+		windows      []*pendingWindow
+		firstAttempt bool
+	}{{first, true}, {retries, false}} {
+		firstAttempt := pass.firstAttempt
+		for _, pw := range pass.windows {
 			if attempts >= cd.maxExportsPerTick || cd.runState.IsStopping() {
 				break
 			}
-			if (pw.attempts == 0) != firstAttempt || pw.nextTick > cd.tickCount {
-				continue
-			}
 			attempts++
 
 			if cd.exportAndLog(opencost.NewClosedWindow(pw.start, pw.start.Add(cd.resolution))) {
@@ -267,6 +282,9 @@ func (cd *ComputeExportController[T]) tick(now time.Time) {
 // retryBackoffTicks returns the number of ticks to wait before retrying a window that has failed the
 // given number of times: 1, 2, 4, 8, then maxRetryBackoffTicks.
 func retryBackoffTicks(attempts int) uint64 {
+	if attempts < 1 {
+		return 1
+	}
 	if attempts > 4 {
 		return maxRetryBackoffTicks
 	}

+ 28 - 0
core/pkg/exporter/controller_pending_test.go

@@ -299,3 +299,31 @@ func TestComputeExportController_ErrorCollectionKeepsWindowPending(t *testing.T)
 		t.Errorf("pendingCount() = %d after recovery, want 0", n)
 	}
 }
+
+// Many windows that always fail must not hold the retry slot ahead of a newer window that failed once:
+// retries go to the fewest-attempted windows first.
+func TestComputeExportController_ManyPoisonedWindowsDoNotBlockRetries(t *testing.T) {
+	transient := at(14, 0, 0)
+	src := &fakeComputeSource[controllerTestSet]{}
+	exp := &fakeComputeExporter[controllerTestSet]{
+		failIf: func(w opencost.Window, now time.Time) bool {
+			s := *w.Start()
+			if s.Before(at(14, 0, 0)) && !s.Before(at(1, 0, 0)) {
+				return true // 13 windows that always fail
+			}
+			return s.Equal(transient) && now.Before(at(15, 10, 0))
+		},
+	}
+	c := NewComputeExportController[controllerTestSet](src, exp, time.Hour)
+
+	runTicks(c, exp, ticksEvery(at(0, 30, 0), at(18, 0, 0), 5*time.Minute), nil)
+
+	i := firstPostCloseSuccess(exp.Records(), transient)
+	if i < 0 {
+		t.Fatalf("window %s was never exported after its transient failure cleared", hourWindow(transient))
+	}
+	// the transient failure clears at 15:10; with backoff it must be retried within maxRetryBackoffTicks
+	if bound := at(15, 10, 0).Add(maxRetryBackoffTicks * 5 * time.Minute); exp.Records()[i].Now.After(bound) {
+		t.Errorf("window %s finalized at %s, after %s", hourWindow(transient), exp.Records()[i].Now.Format("15:04:05"), bound.Format("15:04:05"))
+	}
+}