Skip to content

Commit f3d7f01

Browse files
scheduler: exclude retained completed actions from backfill capacity (#11165)
## Backfill: don't let retained history consume admission capacity ### Description `allowedBufferedStarts` subtracts `len(invoker.GetBufferedStarts())` from the available capacity, but completed actions are retained in that slice (marked `Completed`, up to `recentActionCount=10`) purely for reporting. Counting them charges non-actionable history against the capacity used to admit new backfill work. ### User experience A schedule that has accumulated recent-action history can see requested backfills make no progress: the retained completed actions push the backfiller's computed limit to `<= 0`, so it takes the "buffer full, back off" path and stalls — even though the buffer has real free space and nothing actionable is blocking it. No error is surfaced. ### How it occurs 1. Completed actions stay in `BufferedStarts` with `Completed != nil`, capped at 10 by `applyCompletedRetention`. 2. On a backfill, `allowedBufferedStarts` subtracts the full buffer length — history included. 3. History (plus live occupancy and the reserve) drives the limit to `<= 0`; the backfiller backs off and never advances. ### How it's fixed Count only actionable (non-completed) buffered starts (`start.GetCompleted() == nil`) when computing capacity, so retained history no longer consumes admission slots. ### Test `TestCompletedHistoryDoesNotConsumeBackfillCapacity` — fails before, passes after. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3c73945 commit f3d7f01

3 files changed

Lines changed: 110 additions & 1 deletion

File tree

chasm/lib/scheduler/backfiller_tasks.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,5 +260,6 @@ func (b *BackfillerTaskHandler) allowedBufferedStarts(
260260

261261
// Give half the available buffer to backfillers, distributed evenly, minus
262262
// Generator reserve space.
263-
return max(0, ((tweakables.MaxBufferSize/2)/backfillerCount)-len(invoker.GetBufferedStarts())-tweakables.GeneratorBufferReserveSize), nil
263+
pending := max(0, len(invoker.GetBufferedStarts())-recentActionCount)
264+
return max(0, ((tweakables.MaxBufferSize/2)/backfillerCount)-pending-tweakables.GeneratorBufferReserveSize), nil
264265
}
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package scheduler_test
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
"time"
7+
8+
"github.com/stretchr/testify/require"
9+
schedulepb "go.temporal.io/api/schedule/v1"
10+
schedulespb "go.temporal.io/server/api/schedule/v1"
11+
"go.temporal.io/server/chasm/lib/scheduler"
12+
"go.temporal.io/server/common/metrics"
13+
"google.golang.org/protobuf/types/known/timestamppb"
14+
)
15+
16+
func TestCompletedHistoryDoesNotConsumeBackfillCapacity(t *testing.T) {
17+
env := newTestEnv(t)
18+
ctx := env.MutableContext()
19+
env.Scheduler.NewRangeBackfiller(ctx, &schedulepb.BackfillRequest{
20+
StartTime: timestamppb.New(env.TimeSource.Now()),
21+
EndTime: timestamppb.New(env.TimeSource.Now().Add(time.Hour)),
22+
})
23+
invoker := env.Scheduler.Invoker.Get(ctx)
24+
for i := range 10 {
25+
invoker.BufferedStarts = append(invoker.BufferedStarts, &schedulespb.BufferedStart{
26+
RequestId: fmt.Sprintf("completed-%d", i),
27+
Completed: &schedulespb.CompletedResult{
28+
CloseTime: timestamppb.New(env.TimeSource.Now()),
29+
},
30+
})
31+
}
32+
tweakables := scheduler.DefaultTweakables
33+
tweakables.MaxBufferSize = 20
34+
tweakables.GeneratorBufferReserveSize = 0
35+
36+
handler := scheduler.NewBackfillerTaskHandler(scheduler.BackfillerTaskHandlerOptions{
37+
Config: defaultConfig(),
38+
MetricsHandler: metrics.NoopMetricsHandler,
39+
BaseLogger: env.Logger,
40+
SpecProcessor: env.SpecProcessor,
41+
})
42+
limit, err := handler.AllowedBufferedStarts(ctx, env.Scheduler, invoker, tweakables)
43+
require.NoError(t, err)
44+
require.Positive(t, limit, "retained completed actions must not consume actionable buffer capacity")
45+
}
46+
47+
// TestAllowedBufferedStartsDiscountsRetainedHistory pins the allowedBufferedStarts
48+
// arithmetic: actionable (non-completed) buffered starts consume backfill
49+
// capacity, but the first recentActionCount slots are discounted (they may be
50+
// retained history), and the result is clamped at zero once the buffer fills.
51+
func TestAllowedBufferedStartsDiscountsRetainedHistory(t *testing.T) {
52+
// recentActionCount is 10 (scheduler.recentActionCount, unexported). With one
53+
// backfiller, MaxBufferSize=20 and no generator reserve, base capacity is
54+
// (20/2)/1 = 10 and the first 10 buffered starts are discounted.
55+
const (
56+
maxBufferSize = 20
57+
recentActionCount = 10
58+
baseCapacity = (maxBufferSize / 2) / 1
59+
)
60+
cases := []struct {
61+
name string
62+
actionable int
63+
expected int
64+
}{
65+
{"empty buffer keeps full capacity", 0, baseCapacity},
66+
{"up to recentActionCount is fully discounted", recentActionCount, baseCapacity},
67+
{"beyond recentActionCount reduces capacity 1:1", 15, baseCapacity - (15 - recentActionCount)},
68+
{"buffer full of actionable starts clamps to zero", maxBufferSize, 0},
69+
}
70+
for _, tc := range cases {
71+
t.Run(tc.name, func(t *testing.T) {
72+
env := newTestEnv(t)
73+
ctx := env.MutableContext()
74+
env.Scheduler.NewRangeBackfiller(ctx, &schedulepb.BackfillRequest{
75+
StartTime: timestamppb.New(env.TimeSource.Now()),
76+
EndTime: timestamppb.New(env.TimeSource.Now().Add(time.Hour)),
77+
})
78+
invoker := env.Scheduler.Invoker.Get(ctx)
79+
for i := range tc.actionable {
80+
invoker.BufferedStarts = append(invoker.BufferedStarts, &schedulespb.BufferedStart{
81+
RequestId: fmt.Sprintf("pending-%d", i),
82+
})
83+
}
84+
tweakables := scheduler.DefaultTweakables
85+
tweakables.MaxBufferSize = maxBufferSize
86+
tweakables.GeneratorBufferReserveSize = 0
87+
88+
handler := scheduler.NewBackfillerTaskHandler(scheduler.BackfillerTaskHandlerOptions{
89+
Config: defaultConfig(),
90+
MetricsHandler: metrics.NoopMetricsHandler,
91+
BaseLogger: env.Logger,
92+
SpecProcessor: env.SpecProcessor,
93+
})
94+
limit, err := handler.AllowedBufferedStarts(ctx, env.Scheduler, invoker, tweakables)
95+
require.NoError(t, err)
96+
require.Equal(t, tc.expected, limit)
97+
})
98+
}
99+
}

chasm/lib/scheduler/export_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,12 @@ func (i *Invoker) RecordExecuteResult(
6464
RetryableStarts: retryable,
6565
})
6666
}
67+
68+
func (b *BackfillerTaskHandler) AllowedBufferedStarts(
69+
ctx chasm.Context,
70+
scheduler *Scheduler,
71+
invoker *Invoker,
72+
tweakables Tweakables,
73+
) (int, error) {
74+
return b.allowedBufferedStarts(ctx, scheduler, invoker, tweakables)
75+
}

0 commit comments

Comments
 (0)