Commit db3d52c
[OPIK-8262] [BE] fix: classify provider errors by status and split thread fan-out
Two coupled fixes to online scoring. They ship together because the
second only becomes a correctness problem once the first lands.
Retry classification. scoreTrace answered every provider failure with a
blanket InternalServerErrorException, so a request the provider can
never accept -- an oversized body rejected with a plain-text 400 -- was
replayed maxRetries times, one delivery per pendingMessageDuration,
before being retired. It now classifies by status code rather than by
4xx/5xx family: 400/401/403 and friends become ClientErrorException,
which BaseRedisSubscriber.NON_RETRYABLE_EXCEPTIONS retires on the first
delivery, while 408/425/429 and every 5xx stay a retryable 500. Family
alone is too blunt -- 408/425/429 are "not now", not "not ever".
Permanence is decided only from a status the provider actually put on
the wire. The provider mappers are not consulted, not even as a
fallback: they synthesize a code when they cannot parse the body
(CustomLlm 400, OpenAi 500), and nothing downstream can tell a parsed
400 from that default, so trusting them would drop every unparseable
CustomLlm failure on its first delivery. Needlessly retrying a
permanent error costs maxRetries attempts; dropping an unknown failure
loses it forever, so an absent wire status stays retryable.
VertexAI was the one provider this could not reach. The Google Cloud
SDK raises GAX ApiException, which carries no HttpException and which
langchain4j does not model as NonRetriableException, so a permanent
Vertex failure was retried in-process and then redelivered by the
subscriber. Its status now comes from GAX's own transport-neutral
translation, identical for the gRPC and HTTP-JSON transports, and a
permanent status additionally fails fast inside the retry action so the
in-process attempts are skipped too. An exception GAX itself marks
retryable yields no status, so the mapping can only ever prevent a
drop, never cause one.
Thread fan-out. The two trace-thread scorers took a message carrying a
list of thread ids, fanned out, and collapsed all per-thread errors
into one arbitrary re-emitted error. That was harmless only while every
provider failure was retryable; after the split a fan-out can mix a
permanent 400 with a transient 429, and an arbitrary pick either drops
retryable work or replays threads that already succeeded.
Rather than choose a better victim, remove the collapse: the subscriber
acks and removes per stream entry, so OnlineScorePublisher now writes
one entry per thread id and failure granularity matches ack
granularity. A retry re-runs only the thread that failed, so no sibling
is ever replayed.
Entries left by the previous build carry several ids. Those are
migrated, not scored: the consumer republishes them as N single-id
entries and completes, which is what acks the original. A failed
republish does not ack, so the entry redelivers and the split retries;
a successful republish whose ack fails splits twice, and the duplicate
scores are absorbed by feedback_scores being a ReplacingMergeTree. The
branch is a shim, deletable once no pre-deploy entry can be in flight.
The pipeline lives in OnlineScoringBaseScorer so it cannot drift
between the two scorers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent 9520bc5 commit db3d52c
12 files changed
Lines changed: 1241 additions & 102 deletions
File tree
- apps/opik-backend/src
- main/java/com/comet/opik
- api/resources/v1/events
- domain
- evaluators
- llm
- threads
- test/java/com/comet/opik
- api/resources/v1/events
- domain
- evaluators
- llm
Lines changed: 43 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
6 | 7 | | |
7 | 8 | | |
8 | 9 | | |
| |||
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| 16 | + | |
15 | 17 | | |
16 | 18 | | |
17 | 19 | | |
| |||
31 | 33 | | |
32 | 34 | | |
33 | 35 | | |
| 36 | + | |
34 | 37 | | |
35 | 38 | | |
36 | 39 | | |
| |||
252 | 255 | | |
253 | 256 | | |
254 | 257 | | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
255 | 298 | | |
Lines changed: 13 additions & 22 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
8 | 7 | | |
9 | 8 | | |
10 | 9 | | |
| |||
19 | 18 | | |
20 | 19 | | |
21 | 20 | | |
| 21 | + | |
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
38 | | - | |
39 | 38 | | |
40 | 39 | | |
41 | 40 | | |
| |||
70 | 69 | | |
71 | 70 | | |
72 | 71 | | |
| 72 | + | |
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
| |||
85 | 85 | | |
86 | 86 | | |
87 | 87 | | |
88 | | - | |
| 88 | + | |
| 89 | + | |
89 | 90 | | |
90 | 91 | | |
91 | 92 | | |
| |||
97 | 98 | | |
98 | 99 | | |
99 | 100 | | |
| 101 | + | |
100 | 102 | | |
101 | 103 | | |
102 | 104 | | |
| |||
112 | 114 | | |
113 | 115 | | |
114 | 116 | | |
115 | | - | |
116 | | - | |
117 | | - | |
118 | | - | |
119 | | - | |
120 | | - | |
121 | | - | |
122 | | - | |
123 | | - | |
124 | | - | |
125 | | - | |
126 | | - | |
127 | | - | |
128 | | - | |
129 | | - | |
130 | | - | |
131 | | - | |
132 | | - | |
133 | | - | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
134 | 125 | | |
135 | 126 | | |
136 | 127 | | |
| |||
Lines changed: 13 additions & 22 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
8 | 7 | | |
9 | 8 | | |
10 | 9 | | |
11 | 10 | | |
12 | 11 | | |
13 | 12 | | |
14 | 13 | | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
30 | 29 | | |
31 | 30 | | |
32 | 31 | | |
| |||
62 | 61 | | |
63 | 62 | | |
64 | 63 | | |
| 64 | + | |
65 | 65 | | |
66 | 66 | | |
67 | 67 | | |
| |||
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
78 | | - | |
| 78 | + | |
| 79 | + | |
79 | 80 | | |
80 | 81 | | |
81 | 82 | | |
| |||
85 | 86 | | |
86 | 87 | | |
87 | 88 | | |
| 89 | + | |
88 | 90 | | |
89 | 91 | | |
90 | 92 | | |
| |||
104 | 106 | | |
105 | 107 | | |
106 | 108 | | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | | - | |
115 | | - | |
116 | | - | |
117 | | - | |
118 | | - | |
119 | | - | |
120 | | - | |
121 | | - | |
122 | | - | |
123 | | - | |
124 | | - | |
125 | | - | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
126 | 117 | | |
127 | 118 | | |
128 | 119 | | |
| |||
Lines changed: 4 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
185 | 185 | | |
186 | 186 | | |
187 | 187 | | |
188 | | - | |
189 | | - | |
| 188 | + | |
| 189 | + | |
190 | 190 | | |
191 | 191 | | |
192 | 192 | | |
| |||
384 | 384 | | |
385 | 385 | | |
386 | 386 | | |
387 | | - | |
388 | | - | |
| 387 | + | |
| 388 | + | |
389 | 389 | | |
390 | 390 | | |
391 | 391 | | |
| |||
0 commit comments