Commit 0c5c0ae
Silent-failure ODS counters (#5850)
Summary:
X-link: facebookresearch/FBGEMM#2768
Add OBC counters for 3 silent failure modes in the Raw Embedding Streaming (RES) path, closing the must-have bar of T273802725 (subtask of T269497764, RES code-side observability this half). Today these failure sites only emit `XLOG(ERR)` / `LOG(ERROR)` — nothing aggregates, so on-call cannot trigger off them or query them in ODS without `tail -f` on individual trainer hosts.
Counters added (all via `TrainingPsOdsLogger::bumpKey`, OBC category `raw_embedding_streaming`):
| Counter | Site |
|---|---|
| `res.fail.shard_size_mismatch` | `raw_embedding_streamer.cpp` — both the outer `indices.size(0) != weights.size(0)` guard and the per-shard `weights_masked.size(0) != rows_in_shard` guard in `tensor_stream()` |
| `res.fail.set_embeddings_rpc` | `raw_embedding_streamer.cpp` — wraps `co_await res_client->co_setEmbeddings(req)` in try/catch; bumps then re-throws to preserve trainer-thread termination |
| `res.fail.write_sparse_throw` | `TrainingPsHandler.cpp` `catch (const std::exception& e)` block at the `writeSparseListOfTensors` site in `co_stream_tensors` |
Backend (addresses review: use OBC, not fb303): all three route through the existing `TrainingPsOdsLogger` OBC client. The handler already owned an `ods_logger_`; the fbgemm-side `RawEmbeddingStreamer` now holds a `TrainingPsOdsLogger` member (forward-declared in the OSS-mirrored header, constructed only when streaming is enabled). OBC reaches ODS through the host-level agent with no per-process fb303 export/scrape config, so this revision deletes the fb303 `ResFailCounters.h` helper (and its `call_once` / `addStatExportType` / `FOLLY_EXPORT` machinery) entirely.
Counters bump AT the failure site (inside catch / right before `continue`) so they reflect what actually happened — applying the correctness lesson from D104076551 V4 (don't preemptively count attempts at the top of the function). Per-TBE-per-rank granularity is fine: OBC aggregates per (entity, key), so per-TBE bumps land on the same per-host key rather than separate rows.
Existing log lines preserved (additive). Existing throw-on-failure semantics preserved — the `co_setEmbeddings` catch re-throws after bumping so trainer-thread termination is unchanged.
Out of scope (intentional, per T269497764 boundary):
- MPSC `weights_to_stream_queue_` near-full counter — queue is unbounded; deferred to the queue-depth gauge work in T273802756.
- Detectors / alerts on these counters — downstream consumer work.
Reviewed By: FriedCosey
Differential Revision: D1078115901 parent 80bd3c0 commit 0c5c0ae
3 files changed
Lines changed: 93 additions & 2 deletions
File tree
- fbgemm_gpu
- include/fbgemm_gpu/split_embeddings_cache
- src/split_embeddings_cache
- tests
Lines changed: 12 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
17 | 23 | | |
18 | 24 | | |
19 | 25 | | |
| |||
125 | 131 | | |
126 | 132 | | |
127 | 133 | | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
128 | 140 | | |
129 | 141 | | |
130 | 142 | | |
| |||
Lines changed: 22 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| 13 | + | |
13 | 14 | | |
14 | 15 | | |
15 | 16 | | |
| |||
49 | 50 | | |
50 | 51 | | |
51 | 52 | | |
| 53 | + | |
52 | 54 | | |
53 | 55 | | |
54 | 56 | | |
| |||
167 | 169 | | |
168 | 170 | | |
169 | 171 | | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
170 | 175 | | |
171 | 176 | | |
172 | 177 | | |
| |||
319 | 324 | | |
320 | 325 | | |
321 | 326 | | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
322 | 330 | | |
323 | 331 | | |
324 | 332 | | |
| |||
399 | 407 | | |
400 | 408 | | |
401 | 409 | | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
402 | 413 | | |
403 | 414 | | |
404 | 415 | | |
| |||
420 | 431 | | |
421 | 432 | | |
422 | 433 | | |
423 | | - | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
424 | 445 | | |
425 | 446 | | |
426 | 447 | | |
| |||
Lines changed: 59 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
237 | 237 | | |
238 | 238 | | |
239 | 239 | | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 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 | + | |
240 | 295 | | |
241 | 296 | | |
242 | 297 | | |
| |||
258 | 313 | | |
259 | 314 | | |
260 | 315 | | |
261 | | - | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
262 | 320 | | |
263 | 321 | | |
264 | 322 | | |
| |||
0 commit comments