Skip to content

Commit 08b9a9e

Browse files
committed
fix: add bounded jittered backoff to StartableToolSet retry path
Fixes issue #4060: RAG semantic-embeddings indexing triggered a rate-limit retry storm — repeated toolset-start attempts had no pacing after a 429 from the embedding provider. Implementation: - modelerrors.RetryableHTTPStatus(err): HTTP-status classifier that recognises 429, 408, and 5xx via *StatusError first, then falls back to statusCodeRegex. The toolset gate pre-filters to *StatusError via errors.As before calling it, so port numbers and chunk counts in plain error strings cannot arm the gate. - pkg/tools/startable_backoff.go: bounded exponential backoff with additive 0-20% jitter (base=15s, cap=5min, delay∈[d,1.2d]). - Gate in tryStartLocked (TryStart/TryStartWithTimeout only): blocking Start() bypasses it so mcpcatalog enable and skill startup are immediate. - Gate adopts a live StartReporter after /toolset-restart without waiting for the window to expire. - Wrap embedding errors in WrapHTTPError at openai/client.go and dmr/embed.go so a 429 from the embedding provider surfaces as *StatusError and correctly arms the gate. - WithStartRetryJitter / WithStartRetryClock options via variadic NewStartable for deterministic test control. - Stale 'retry on next turn' log messages updated in agent.go/mcp.go. - Partial-start exemption documented (code-mode composites remain unpaced; follow-up at issue #4067). Tests (same commit, covering the above): - startable_backoff_test.go: unit tests for the gate (gate fires on 429/408/5xx StatusError, not on plain text / context errors, blocking Start() ungated, concurrency, jitter bounds) - startable_backoff_regression_test.go: consumer-shaped regression suite (RAG/MCP/LSP error shapes, no-goroutine/timer leak, latch) - rag_backoff_test.go: real-toolset integration test via rag.New + fake clock Docs: - docs/tools/rag/index.md: 'Indexing failures, retries and backoff' section with trigger table, parameters, and troubleshooting. - docs/tools/mcp/index.md, docs/tools/lsp/index.md: lifecycle notes confirming local startup failures fail fast. Scope: DefaultStartTimeout (30s) unchanged — deferred.
1 parent b407a13 commit 08b9a9e

18 files changed

Lines changed: 1689 additions & 13 deletions

File tree

docs/community/troubleshooting/index.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ MCP tools using stdio transport must complete the initialization handshake befor
199199
200200
If a toolset keeps crashing in a tight loop, tune the [`lifecycle`](../../configuration/tools/index.md#toolset-lifecycle) block on the toolset (e.g. raise `backoff.initial`, lower `max_restarts`, or switch to the `best-effort` profile) so a flaky dependency does not amplify into a restart storm.
201201

202+
If a **RAG knowledge base** is failing to index because the embedding provider is rate-limiting requests (HTTP 429), Docker Agent automatically backs off and retries — see [Indexing failures, retries and backoff](../../tools/rag/index.md#indexing-failures-retries-and-backoff) for the retry schedule and the `max_indexing_concurrency` / `max_embedding_concurrency` knobs that control how much concurrent load is generated.
203+
202204
## Configuration Errors
203205

204206
### YAML syntax issues

docs/tools/lsp/index.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ Available Capabilities:
197197

198198
LSP toolsets are managed by the same supervisor as MCP toolsets, so a crashed `gopls` (or any other language server) is reconnected automatically with exponential backoff. Use the [`lifecycle`](../../configuration/tools/index.md#toolset-lifecycle) block to tune the policy per toolset — for example, mark `gopls` as `strict` if your CI flow requires it to be available, or use `/toolset-restart gopls` from the TUI to force a reconnect when the server gets stuck.
199199

200+
**Startup failure behaviour:** local LSP server startup failures (missing binary, server-unavailable) fail fast — each turn retries immediately with no artificial delay. The rate-limit backoff gate applies only to model-provider embedding calls (see [Indexing failures, retries and backoff](../rag/index.md#indexing-failures-retries-and-backoff)); it does not apply to LSP server startup.
201+
200202
```yaml
201203
toolsets:
202204
- type: lsp

docs/tools/mcp/index.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,8 @@ toolsets:
258258

259259
See [Toolset Lifecycle](../../configuration/tools/index.md#toolset-lifecycle) for all profiles and tuning knobs, and [`/toolset-restart`](../../features/tui/index.md) to force a reconnect from the TUI.
260260

261+
**Startup failure behaviour:** local MCP startup failures (missing binary, connection refused, authentication error) fail fast — each turn retries immediately with no artificial delay. The rate-limit backoff gate applies only to model-provider embedding calls (see [Indexing failures, retries and backoff](../rag/index.md#indexing-failures-retries-and-backoff)); it does not apply to MCP server startup.
262+
261263
## Combined Example
262264

263265
```yaml

docs/tools/rag/index.md

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,92 @@ chunking:
174174
>
175175
> Currently supports Go (`.go`) files. More languages will be added. Falls back to plain text chunking for unsupported file types.
176176

177+
## Indexing failures, retries and backoff
178+
179+
When a knowledge-base fails to start — because the embedding provider is rate-limiting
180+
your requests or returning a transient server error — Docker Agent spaces out retry
181+
attempts with bounded exponential backoff instead of hammering the provider on
182+
every agent turn.
183+
184+
### What triggers backoff
185+
186+
Backoff applies only to **HTTP 429 rate-limit** responses from the embedding or
187+
model provider — the one signal that reliably reaches the toolset gate. Other
188+
errors (5xx, 408) are handled per-file within the indexing run and do not arm
189+
the gate. These are the current gate triggers:
190+
191+
| Failure kind | Behaviour |
192+
|---|---|
193+
| HTTP 429 (rate limit) | Backoff: next attempt delayed |
194+
| Other failures (5xx, 408, config errors, auth) | Fail fast: retried every turn with no added delay |
195+
| Context cancellation or agent shutdown | Immediate: no delay |
196+
197+
> [!NOTE]
198+
> 5xx and 408 errors from the embedding provider are retried per-file and do not
199+
> propagate to the toolset gate. Only 429 (rate-limit) terminates the indexing run
200+
> early and surfaces the gate so Docker Agent can pace the next attempt.
201+
202+
### Retry policy and parameters
203+
204+
The backoff is **bounded exponential with additive jitter**:
205+
206+
- **Base delay**: 15 seconds
207+
- **Maximum delay**: up to ~6 minutes (5-minute cap plus up to 20% additive jitter)
208+
- **Growth**: doubles after each consecutive retryable failure (15s → 30s → 1m → 2m → 4m → 5m)
209+
- **Jitter**: each wait is a random value in `[nominal, 1.2×nominal]` (additive 0–20%)
210+
so concurrent knowledge-base sources spread their retries and avoid
211+
hammering the provider together
212+
- **Retry-After override**: if the embedding provider responds with a `Retry-After`
213+
header, that hint overrides the computed delay (capped at the 5-minute maximum,
214+
with the same additive jitter applied to spread concurrent retries)
215+
216+
The gate is a lightweight wall-clock check — it creates no background threads or
217+
timers. A Stop command or agent shutdown takes effect immediately regardless of
218+
how much of the backoff window remains.
219+
220+
### Operational impact
221+
222+
**Before**: a rate-limited knowledge base was re-indexed on every agent turn —
223+
`max_indexing_concurrency × max_embedding_concurrency` concurrent provider calls
224+
could relaunch within milliseconds, easily tripping rate limits for both the
225+
knowledge base and the agent's own model calls.
226+
227+
**After**: retries are spaced out and jittered so the provider has room to recover
228+
before the next attempt. The agent continues working with any other toolsets that
229+
are not affected.
230+
231+
### What you will see
232+
233+
- Docker Agent logs a single warning when a knowledge base first fails to start.
234+
Repeated failures in between are logged at debug level only, so you are not
235+
flooded with alerts on every turn. Recovery is intentionally silent — the
236+
tool appearing in the agent's tool list is the signal that indexing succeeded.
237+
- The knowledge-base tool does not appear in the agent's tool list until indexing
238+
succeeds. A successful start is silent — the tool is listed and the agent uses it.
239+
240+
### Troubleshooting repeated 429 errors
241+
242+
If you see persistent `429` errors in the logs:
243+
244+
1. **Check provider rate limits.** Your embedding API key may have a low requests-per-minute
245+
quota. Upgrading the plan or using a different API key can help.
246+
2. **Reduce concurrency.** The chunked-embeddings and semantic-embeddings strategies
247+
accept `max_indexing_concurrency` (default `3`) and `max_embedding_concurrency`
248+
(default `3`) parameters. Lowering these reduces simultaneous requests:
249+
250+
```yaml
251+
rag:
252+
docs:
253+
docs: [./knowledge-base]
254+
strategies:
255+
- type: chunked-embeddings
256+
max_indexing_concurrency: 1
257+
max_embedding_concurrency: 1
258+
```
259+
260+
3. **Use a model with a higher quota.** Some providers offer higher rate limits on
261+
specific embedding model tiers.
262+
177263
## Debugging RAG
178264

179265
Enable debug logging to see retrieval details:
@@ -218,6 +304,7 @@ Look for log tags: `[RAG Manager]`, `[Chunked-Embeddings Strategy]`, `[BM25 Stra
218304
| `limit` | int | `5` | Max results from this strategy |
219305
| `embedding_batch_size` | int | `50` | Chunks per embedding request |
220306
| `max_embedding_concurrency` | int | `3` | Max concurrent embedding requests |
307+
| `max_indexing_concurrency` | int | `3` | Max concurrent file-indexing tasks |
221308
| `chunking.size` | int | `1500` | Chunk size in characters (`4000` when `code_aware` is set) |
222309
| `chunking.overlap` | int | `75` | Overlap between chunks in characters |
223310
| `chunking.code_aware` | bool | `false` | AST-based chunking (Go files only) |

pkg/agent/agent.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -645,10 +645,10 @@ func (a *Agent) ensureToolSetsAreStarted(ctx context.Context) {
645645
continue
646646
}
647647
if toolSet.ShouldReportFailure() {
648-
slog.WarnContext(ctx, "Toolset start failed; will retry on next turn", "agent", a.Name(), "toolset", desc, "error", err)
648+
slog.WarnContext(ctx, "Toolset start failed; will retry (backoff may apply)", "agent", a.Name(), "toolset", desc, "error", err)
649649
a.AddToolWarning(fmt.Sprintf("%s start failed: %v", desc, err))
650650
} else {
651-
slog.DebugContext(ctx, "Toolset still unavailable; retrying next turn", "agent", a.Name(), "toolset", desc, "error", err)
651+
slog.DebugContext(ctx, "Toolset still unavailable; will retry (backoff may apply)", "agent", a.Name(), "toolset", desc, "error", err)
652652
}
653653
}
654654
}

pkg/model/provider/dmr/embed.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/openai/openai-go/v3"
1818

1919
"github.com/docker/docker-agent/pkg/model/provider/base"
20+
"github.com/docker/docker-agent/pkg/model/provider/oaistream"
2021
"github.com/docker/docker-agent/pkg/rag/types"
2122
)
2223

@@ -50,7 +51,7 @@ func (c *Client) CreateBatchEmbedding(ctx context.Context, texts []string) (*bas
5051
Model: c.ModelConfig.Model,
5152
})
5253
if err != nil {
53-
return nil, fmt.Errorf("failed to create embeddings: %w", err)
54+
return nil, fmt.Errorf("failed to create embeddings: %w", oaistream.WrapOpenAIError(err))
5455
}
5556

5657
if len(response.Data) != len(texts) {
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package dmr
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
openaisdk "github.com/openai/openai-go/v3"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/docker/docker-agent/pkg/config/latest"
13+
"github.com/docker/docker-agent/pkg/modelerrors"
14+
)
15+
16+
// TestCreateBatchEmbedding_429SurfacesAsStatusError proves that when the
17+
// embedding endpoint returns HTTP 429 the error propagates as
18+
// *modelerrors.StatusError so the StartableToolSet backoff gate can arm.
19+
func TestCreateBatchEmbedding_429SurfacesAsStatusError(t *testing.T) {
20+
t.Parallel()
21+
22+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
23+
w.Header().Set("Content-Type", "application/json")
24+
w.WriteHeader(http.StatusTooManyRequests)
25+
_, _ = w.Write([]byte(`{"error":{"message":"rate limit exceeded","type":"requests","code":"rate_limit_exceeded"}}`))
26+
}))
27+
t.Cleanup(srv.Close)
28+
29+
cfg := &latest.ModelConfig{
30+
Provider: "dmr",
31+
Model: "ai/nomic-embed-text",
32+
BaseURL: srv.URL + "/v1",
33+
}
34+
client, err := NewClient(t.Context(), cfg, nil)
35+
require.NoError(t, err)
36+
37+
_, err = client.CreateBatchEmbedding(t.Context(), []string{"hello"})
38+
require.Error(t, err, "expected error from 429 response")
39+
40+
// The critical assertion: the OpenAI SDK error must be wrapped in
41+
// *modelerrors.StatusError by WrapOpenAIError so the backoff gate can arm.
42+
var se *modelerrors.StatusError
43+
require.ErrorAs(t, err, &se,
44+
"error must contain *modelerrors.StatusError so the backoff gate can arm")
45+
assert.Equal(t, http.StatusTooManyRequests, se.StatusCode)
46+
47+
// Cross-check: the underlying SDK error is also reachable.
48+
var apiErr *openaisdk.Error
49+
assert.ErrorAs(t, err, &apiErr,
50+
"underlying *openaisdk.Error must be reachable through the chain")
51+
}

pkg/model/provider/openai/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1165,7 +1165,7 @@ func (c *Client) CreateBatchEmbedding(ctx context.Context, texts []string) (*bas
11651165
response, err := client.Embeddings.New(ctx, params)
11661166
if err != nil {
11671167
slog.ErrorContext(ctx, "OpenAI batch embedding request failed", "error", err)
1168-
return nil, fmt.Errorf("failed to create batch embeddings: %w", err)
1168+
return nil, fmt.Errorf("failed to create batch embeddings: %w", oaistream.WrapOpenAIError(err))
11691169
}
11701170

11711171
if len(response.Data) != len(texts) {
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package openai
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
openaisdk "github.com/openai/openai-go/v3"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/docker/docker-agent/pkg/config/latest"
13+
"github.com/docker/docker-agent/pkg/modelerrors"
14+
)
15+
16+
// TestCreateBatchEmbedding_429SurfacesAsStatusError mirrors the DMR embed test
17+
// for the OpenAI provider path (pkg/model/provider/openai/client.go:1168).
18+
// It proves that a 429 from the embedding endpoint propagates as
19+
// *modelerrors.StatusError so the StartableToolSet backoff gate can arm.
20+
func TestCreateBatchEmbedding_429SurfacesAsStatusError(t *testing.T) {
21+
t.Parallel()
22+
23+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
24+
w.Header().Set("Content-Type", "application/json")
25+
w.WriteHeader(http.StatusTooManyRequests)
26+
_, _ = w.Write([]byte(`{"error":{"message":"rate limit exceeded","type":"requests","code":"rate_limit_exceeded"}}`))
27+
}))
28+
t.Cleanup(srv.Close)
29+
30+
cfg := &latest.ModelConfig{
31+
Provider: "openai",
32+
Model: "text-embedding-3-small",
33+
BaseURL: srv.URL,
34+
ProviderOpts: map[string]any{"api_type": "openai_chatcompletions"}, // custom provider → no auth required
35+
}
36+
client, err := NewClient(t.Context(), cfg, nil)
37+
require.NoError(t, err)
38+
39+
_, err = client.CreateBatchEmbedding(t.Context(), []string{"hello"})
40+
require.Error(t, err, "expected error from 429 response")
41+
42+
// The critical assertion: the error must contain *modelerrors.StatusError
43+
// so the StartableToolSet backoff gate can arm.
44+
var se *modelerrors.StatusError
45+
require.ErrorAs(t, err, &se,
46+
"error must wrap *modelerrors.StatusError so the backoff gate can arm")
47+
assert.Equal(t, http.StatusTooManyRequests, se.StatusCode)
48+
49+
// The underlying SDK error must also be reachable.
50+
var apiErr *openaisdk.Error
51+
assert.ErrorAs(t, err, &apiErr,
52+
"underlying *openaisdk.Error must be reachable through the chain")
53+
}
54+
55+
// TestCreateBatchEmbedding_5xxDoesNotSurfaceAsStatusError verifies that 5xx
56+
// errors from the embedding endpoint are handled but do NOT propagate as
57+
// *modelerrors.StatusError (they are per-file retryable errors swallowed by
58+
// the indexing strategy, not gate-arming errors).
59+
func TestCreateBatchEmbedding_5xxSurfacesAsStatusError(t *testing.T) {
60+
t.Parallel()
61+
62+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
63+
w.Header().Set("Content-Type", "application/json")
64+
w.WriteHeader(http.StatusServiceUnavailable)
65+
_, _ = w.Write([]byte(`{"error":{"message":"service unavailable","type":"server_error"}}`))
66+
}))
67+
t.Cleanup(srv.Close)
68+
69+
cfg := &latest.ModelConfig{
70+
Provider: "openai",
71+
Model: "text-embedding-3-small",
72+
BaseURL: srv.URL,
73+
ProviderOpts: map[string]any{"api_type": "openai_chatcompletions"}, // custom provider → no auth required
74+
}
75+
client, err := NewClient(t.Context(), cfg, nil)
76+
require.NoError(t, err)
77+
78+
_, err = client.CreateBatchEmbedding(t.Context(), []string{"hello"})
79+
require.Error(t, err)
80+
81+
// 5xx is also wrapped by WrapOpenAIError — it surfaces as *StatusError
82+
// but is classified retryable-per-file by classifyModelCallError.
83+
var se *modelerrors.StatusError
84+
require.ErrorAs(t, err, &se)
85+
assert.Equal(t, http.StatusServiceUnavailable, se.StatusCode)
86+
87+
// Verify the chain is intact.
88+
var apiErr *openaisdk.Error
89+
assert.ErrorAs(t, err, &apiErr)
90+
}

pkg/modelerrors/modelerrors.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,3 +781,18 @@ func scalarString(v any) string {
781781
return fmt.Sprint(v)
782782
}
783783
}
784+
785+
// RetryableHTTPStatus reports whether err contains an HTTP status that warrants
786+
// backoff (429 rate-limit, 408 request-timeout, or a 5xx server error). It
787+
// checks for a *StatusError in the chain first; if none is found it falls back
788+
// to matching \b([45]\d{2})\b in the error message, which can produce false
789+
// positives for port numbers or similar numeric patterns. Callers that need
790+
// strict StatusError-only classification should pre-filter with errors.As.
791+
// No context-error handling is performed.
792+
func RetryableHTTPStatus(err error) bool {
793+
code := extractHTTPStatusCode(err)
794+
if code == 0 {
795+
return false
796+
}
797+
return code == http.StatusTooManyRequests || isRetryableStatusCode(code)
798+
}

0 commit comments

Comments
 (0)