With the flag on, one failed delete breaks the vLLM worker for the rest of the run

Bug in NVIDIA-NeMo/RL #4655. All permalinks pin the PR head 495ce13. Review comments are in a pending review; their links work once it is published.

Before
A failed delete of old chunk rows is logged, and the call still gets its reply. The only cost is a few extra rows in TQ.
This PR
With generation_prefix_cleanup.enabled, the background thread stores the first delete error and exits. From then on, every finishing call and every checkpoint on that worker raises.
Status
Open — not fixed in this PR. Only when the flag is on (it is off by default). Raised in the review; reproduced with the real module.

Two pages on this PR — this one: what one failed delete does with the flag on.

Every finishing call first asks the cleanup thread for a slot with reserve(), even a call that was never cut and has nothing to delete. When a CLEAR fails, the thread stores the error and returns (L206-L215), and reserve() checks that error first (L146-L152). That check sits outside the handler that says "completion is already durable" (L1563), and after the try that cleans up a failed call (L2558-L2560). So the client gets HTTP 500, and the call's capture state stays registered.

call A finishes 2 old chunk rows CLEAR of A's rows TQ times out once call B finishes never cut: 0 rows call C finishes never cut: 0 rows next checkpoint save TQ flag off today's default reply 200 CLEAR inline error logged keeps serving reply 200 reply 200 saved flag on this PR reply 200 CLEAR queued error stored thread exits HTTP 500 reserve() raises HTTP 500 and every call after checkpoint fails wait_paused raises worker keeps working broken until the actor restarts — B and C had nothing to delete 
thread alive after failure: False / pending after failure: 1
reserve raised: RuntimeError('background prefix cleanup failed; no automatic retry')   (every call)
fence wait_paused raised: RuntimeError('background prefix cleanup failed; no automatic retry')

Output of a repro with the real module and a clear that fails once. Only an actor restart clears the stored error.

review comment — background_prefix_cleanup.py:206

The fix — proposed, not in the PR

Nothing below is implemented, and it has not been run.

Where the fix lands: RL only, one file. BackgroundPrefixCleanup._drain, plus dropping the error check from reserve() and wait_paused(). No Gym change.

@@ background_prefix_cleanup.py L206-L216 · _drain
             try:
                 self._clear(keys)
-            except Exception as error:  # noqa: BLE001 - surface arbitrary transport failures
-                with self._condition:
-                    self._error = error
-                    self._failed_keys = keys
-                    self._active = False
-                    self._condition.notify_all()
-                LOGGER.exception(
-                    "background prefix cleanup failed for %d keys", len(keys)
-                )
-                return
+            except Exception:  # noqa: BLE001 - a lost delete only leaves extra rows
+                # Never retry: a CLEAR may have partly succeeded. Keep serving.
+                LOGGER.exception(
+                    "background prefix cleanup failed for %d keys", len(keys)
+                )
             with self._condition:
                 self._active = False
                 self._pending -= len(batch)
                 self._condition.notify_all()

What it does: a failed batch is logged and dropped, the slot count goes back down, and the thread keeps going, exactly like the flag-off path. It never retries, because a CLEAR can partly succeed. A broken TQ still fails loudly at the next final-row write, so nothing is hidden. If the module is rewritten to take a permit on the existing checkpoint gate (previous page), this bug goes away with it.

So what. One transient TQ error turns a perf option into an outage of that generation worker: its rollouts fail with HTTP 500 and no checkpoint can be saved until the actor restarts. The PR's own limits section says a lost delete "leaves extra obsolete rows", so the flag-on path should be no stricter than flag off. The review also asks for a worker-level test: one failed CLEAR, then a second call that must succeed.