docs: both failure modes measured, and whether absorbing an OOM recovers

Prompted by two review questions I could not answer from the code, so I
measured instead of reasoning. Both changed what the javadoc claims.

1. Is the OPIK-8164 failure an OOM? NO. Through the shipped JAVA codec's
   value decoder at the real 20,000,000 limit:
     19,999,999 chars -> decodes fine
     20,000,001 chars -> StreamConstraintsException, isError=false
   An ordinary IOException, matching the production stack trace verbatim.
   The Exception arm has always handled the literal incident; my previous
   commit framed the OOM arm as if it closed a hole related to it, which
   was wrong.

2. But both modes are real, on the SAME path. A payload UNDER
   maxStringLength but larger than the heap OOMs inside Jackson's own
   String materialization. On a -Xmx64m fork, 12,000,000 chars (well under
   the 20,000,000 limit) gave cause=java.lang.OutOfMemoryError,
   isError=true, absorbed into the sentinel. This is the case that makes
   the OOM arm matter in production, where maxStringLength ships at 100 MB
   and consumerBatchSize is 10.

3. Does absorbing an OOM recover? For this shape, yes, measured: three
   consecutive rounds of oversized-then-ordinary on a 64 MB heap each
   absorbed the OOM and then decoded the ordinary message correctly, free
   heap stable, no cumulative degradation. The array that failed was never
   allocated, so the failure consumed nothing.

   The operational argument settles it: the helm chart ships
   -XX:+UseG1GC -XX:MaxRAMPercentage=80.0 with NO
   -XX:+ExitOnOutOfMemoryError, so the process already survives an OOM
   today. Not absorbing does not buy a clean restart -- it buys a
   still-running pod with a permanently wedged stream. If the team ever
   adds ExitOnOutOfMemoryError the JVM exits at throw time and this arm
   becomes unreachable, which is fine.

   Documented what it does NOT claim: a JVM under genuine heap exhaustion
   is not healthy, this arm can absorb an OOM that was a symptom rather
   than a cause, and the recovery measured is single-threaded.

Tests: the over-limit case now asserts the cause is StreamConstraintsException
and is NOT an Error, which is the distinction both questions turned on. Also
removed a shipped-codec test I had just added -- it needed an assumeTrue on
the memoized limit and skipped in the real suite, because RedisStreamCodecTest
calls JsonUtils.configure in the same JVM. The small-limit mapper exercises
the identical path; the real-limit numbers live in the javadoc as measured
evidence instead.

58 green, 0 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: thiagohora

apps/opik-backend/src/main/java/com/comet/opik/infrastructure/redis/RedisStreamCodec.java

@@ -90,40 +90,77 @@ static Codec faultTolerant(Codec delegate) {
      * general wrapper, not a JAVA-specific one, and a three-arg rewiring would otherwise silently
      * reopen the hole.</li>
      * </ul>
-     * The catch covers {@link Exception} plus {@link OutOfMemoryError}, and nothing else. The OOM arm is
-     * not defensive garnish — it closes a real hole. {@code LZ4CodecV2$1.decode} reads a 4-byte declared
-     * decompressed length off the wire and immediately does {@code newarray byte} of exactly that size,
-     * building the decompressor and validating the frame only afterwards (Redisson 4.7.0 bytecode:
-     * {@code readInt} at 1, {@code newarray} at 6, {@code BlockLZ4CompressorInputStream} at 24,
-     * {@code readFully} at 44). A corrupted field-name frame can therefore claim any 31-bit length. What
-     * that produces depends on the heap, which is why it cannot be left to {@link Exception} alone:
+     * The catch covers {@link Exception} plus {@link OutOfMemoryError}, and nothing else. Both arms are
+     * load-bearing, for different failures on different paths, and the OOM arm is <em>not</em> only a
+     * defence against corrupted frames — it also covers the primary payload path. All four rows below
+     * were measured through {@link #JAVA}'s own decoders, not reasoned about:
      *
      * <table border="1">
-     * <caption>Raw {@code LZ4CodecV2} outcome by declared length, measured on a 9 GB heap</caption>
-     * <tr><th>Declared length</th><th>Outcome</th></tr>
-     * <tr><td>negative</td><td>{@code NegativeArraySizeException} — an Exception</td></tr>
-     * <tr><td>large but allocatable</td><td>allocates, then {@code IOException} — an Exception</td></tr>
-     * <tr><td>beyond the heap</td><td>{@link OutOfMemoryError} — an Error</td></tr>
+     * <caption>Observed failure modes</caption>
+     * <tr><th>Input</th><th>Path</th><th>Result</th><th>Arm</th></tr>
+     * <tr><td>String over {@code maxStringLength}</td><td>value</td>
+     *     <td>{@code StreamConstraintsException} — the literal OPIK-8164 failure</td><td>Exception</td></tr>
+     * <tr><td>String under the limit, over the heap</td><td>value</td>
+     *     <td>{@link OutOfMemoryError} inside Jackson's String materialization</td><td><b>OOM</b></td></tr>
+     * <tr><td>Negative LZ4 declared length</td><td>field name</td>
+     *     <td>{@code NegativeArraySizeException}</td><td>Exception</td></tr>
+     * <tr><td>LZ4 declared length beyond the heap</td><td>field name</td>
+     *     <td>{@link OutOfMemoryError} from {@code newarray byte}</td><td><b>OOM</b></td></tr>
      * </table>
      *
-     * Only the last row needs the OOM arm, and which row a given length lands in moves with
-     * {@code -Xmx}: 1.8 GiB is an {@code IOException} on a 9 GB heap and an {@link OutOfMemoryError} on a
-     * 1 GB container. Absorbing it makes the behaviour heap-independent. Without it that row is the
-     * OPIK-8164 wedge again, reached through a corrupted length header instead of an oversized string.
+     * The second row is the one that makes the OOM arm matter in production rather than in theory.
+     * {@code jacksonConfig.maxStringLength} ships at 100 MB, so a payload well inside the configured
+     * limit can still exhaust a container heap while Jackson materializes it — and at
+     * {@code consumerBatchSize: 10} ten such decodes run concurrently. Verified on a {@code -Xmx64m}
+     * fork: a 12,000,000-character payload, comfortably under the 20,000,000 default limit, produced
+     * {@code cause=java.lang.OutOfMemoryError}. Without this arm that Error escapes into
+     * {@code CommandDecoder} with no {@code StreamMessageId} and wedges the stream — the same outcome
+     * OPIK-8164 produced, reached by running out of heap rather than by breaching a limit.
      * <p>
-     * Two honest limits on this. The failing array is unreferenced the moment we return, so the JVM
+     * Rows three and four are the LZ4 field-name path. {@code LZ4CodecV2$1.decode} reads a 4-byte
+     * declared decompressed length and immediately does {@code newarray byte} of that size, building the
+     * decompressor and validating the frame only afterwards (Redisson 4.7.0: {@code readInt} at 1,
+     * {@code newarray} at 6, {@code BlockLZ4CompressorInputStream} at 24, {@code readFully} at 44). Which
+     * of the two arms catches a given length depends on {@code -Xmx}: 1.8 GiB is an {@code IOException}
+     * on a 9 GB heap and an {@link OutOfMemoryError} on a 1 GB container. Absorbing both makes the
+     * behaviour heap-independent.
+     * <p>
+     * Two honest limits. The failing allocation is unreferenced the moment we return, so the JVM
      * recovers — but an allocation large enough to fail can starve a <em>different</em> thread, which
-     * then throws where nothing catches it; and a length the heap can satisfy is still allocated in full
-     * before it fails. Bounding the declared length before the allocation would fix both, and an earlier
-     * revision of this class did exactly that. It was removed because the ceiling was an undocumented
-     * number: nothing in Redis or Redisson constrains a stream field name, so any bound rests on the
-     * local observation that every field name in this codebase is the constant {@code "message"} — an
-     * assumption that silently discards data the day someone picks a longer one. A pre-check belongs
-     * with the {@link CompositeCodec} argument-order fix, which removes LZ4 from the field-name path and
-     * dissolves this hazard rather than bounding it.
+     * then throws where nothing catches it; and a size the heap can satisfy is still allocated in full
+     * before it fails. Bounding the LZ4 declared length pre-allocation would narrow row four, and an
+     * earlier revision did that; it was removed because the ceiling was an undocumented number resting
+     * on the local observation that every field name here is the constant {@code "message"}. Row four
+     * is better fixed by the {@link CompositeCodec} argument-order change, which takes LZ4 off the
+     * field-name path entirely. Row two has no such escape: materializing a large document is the
+     * legitimate work, so absorbing the OOM is the only option short of a size guard at publish time.
+     * <p>
+     * <h4>Is absorbing an {@link OutOfMemoryError} recoverable?</h4>
+     *
+     * For this failure shape, yes, and it was measured rather than assumed. On a {@code -Xmx64m} fork,
+     * three consecutive rounds of "oversized payload, then an ordinary message" each absorbed the OOM
+     * and then decoded the ordinary message correctly, with free heap stable across rounds — no
+     * cumulative degradation. That holds because the array which failed to allocate was never
+     * allocated, so the failure consumed nothing, and the oversized buffer is unreferenced the moment
+     * this method returns.
+     * <p>
+     * The operational comparison is the part that settles it. The shipped JVM options
+     * ({@code -XX:+UseG1GC -XX:MaxRAMPercentage=80.0} in the helm chart) do <em>not</em> include
+     * {@code -XX:+ExitOnOutOfMemoryError}, so the process already survives an OOM today. Not absorbing
+     * therefore does not buy a clean restart — it buys a still-running pod with a permanently wedged
+     * stream, which is strictly worse than dropping one message. Should the team ever add
+     * {@code ExitOnOutOfMemoryError}, the JVM exits at throw time and this arm simply becomes
+     * unreachable; nothing here depends on it staying absent.
+     * <p>
+     * What this does <em>not</em> claim: that a JVM under genuine heap exhaustion is healthy. If the
+     * heap is being exhausted by other work, this arm will absorb an OOM that was a symptom rather than
+     * a cause, and keep consuming — masking it. The recovery measured above is single-threaded and
+     * proves the decode path, not a loaded service; an allocation large enough to fail can still starve
+     * a different thread, which throws where nothing catches it. The counter to that is a size guard at
+     * publish time, which is what {@code onlineScoring.dropOversizedPayloads} does, not anything this
+     * codec can do on read.
      * <p>
-     * No other {@link Error} is absorbed: an {@link OutOfMemoryError} from a legitimate
-     * multi-hundred-MB payload document, or a {@link StackOverflowError}, still propagates.
+     * No other {@link Error} is absorbed — a {@link StackOverflowError} still propagates.
      */
     @RequiredArgsConstructor
     private static final class FaultTolerantCodec implements Codec {

apps/opik-backend/src/test/java/com/comet/opik/infrastructure/redis/FaultTolerantStreamCodecTest.java

@@ -1,6 +1,7 @@
 package com.comet.opik.infrastructure.redis;
 
 import com.fasterxml.jackson.core.StreamReadConstraints;
+import com.fasterxml.jackson.core.exc.StreamConstraintsException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
@@ -32,7 +33,18 @@ class FaultTolerantStreamCodecTest {
 
     private static final int SMALL_STRING_LIMIT = 64;
 
-    /** A mapper whose string limit is small enough to breach without allocating megabytes. */
+    /**
+     * A mapper whose string limit is small enough to breach without allocating megabytes.
+     * <p>
+     * This is the same code path as the shipped codec's value decoder --
+     * {@code faultTolerant(JsonJacksonCodec).getMapValueDecoder()} either way, since
+     * {@code CompositeCodec} routes map values to {@code JsonJacksonCodec} here -- with only the
+     * mapper's limit differing. Deliberately not driven through {@code RedisStreamCodec.JAVA} at its
+     * real limit: the enum memoizes a copy of {@code JsonUtils}' mapper at build time, and
+     * {@code RedisStreamCodecTest} calls {@code JsonUtils.configure} in the same JVM, so the effective
+     * limit there is order-dependent and a test asserting it either skips or flakes. The real-limit
+     * behaviour is recorded as measured evidence in {@code FaultTolerantCodec}'s javadoc instead.
+     */
     private static Codec codecWithSmallStringLimit() {
         var mapper = new ObjectMapper();
         mapper.getFactory().setStreamReadConstraints(
@@ -56,7 +68,12 @@ void oversizedPayloadYieldsSentinel() throws IOException {
         assertThat(decoded).isInstanceOf(UndecodableStreamMessage.class);
         var undecodable = (UndecodableStreamMessage) decoded;
         assertThat(undecodable.encodedBytes()).isEqualTo(encodedBytes);
+        // The type matters, not just the text: this is the OPIK-8164 failure, and it is an ordinary
+        // IOException, NOT an Error. Whether the Exception arm alone suffices for the incident turns on
+        // exactly this -- see heapExhaustionDuringMaterializationIsAbsorbed for the case where it does not.
         assertThat(undecodable.cause())
+                .isInstanceOf(StreamConstraintsException.class)
+                .isNotInstanceOf(Error.class)
                 .hasMessageContaining("maximum allowed");
         // The size is reported from the buffer, so it survives the failed decode consuming it.
         assertThat(buf.isReadable()).isFalse();
@@ -186,12 +203,24 @@ void partiallyConsumedBufferIsDrained() throws IOException {
     }
 
     /**
-     * The OOM arm, driven directly rather than through LZ4's allocation, so it is pinned independently
-     * of any heap size. See {@code corruptedDeclaredLengthYieldsSentinel} for why it is absorbed.
+     * Why the {@link OutOfMemoryError} arm is not only about corrupted LZ4 lengths: it also covers the
+     * PRIMARY payload path. A payload <em>under</em> {@code maxStringLength} but larger than the heap
+     * OOMs inside Jackson's own String materialization, before any constraint is breached.
+     * <p>
+     * Measured, not reasoned: on a fork with {@code -Xmx64m}, a 12,000,000-character payload (well
+     * under the 20,000,000 limit) through {@code RedisStreamCodec.JAVA}'s value decoder yields
+     * {@code SENTINEL, cause=java.lang.OutOfMemoryError, isError=true}. This matters in production,
+     * where {@code maxStringLength} is 100 MB and {@code consumerBatchSize} is 10 -- ten concurrent
+     * materializations of a large trace is exactly this case, and without this arm the OOM escapes into
+     * {@code CommandDecoder} and wedges the stream.
+     * <p>
+     * Driven here through a stub rather than a real allocation, because the surefire JVM's heap is far
+     * too large to provoke it and forcing a small {@code -Xmx} for one test would slow every other one.
+     * The real-heap reproduction above is the evidence; this pins the arm.
      */
     @Test
     @DisplayName("an OutOfMemoryError from a decoder is absorbed into the sentinel")
-    void outOfMemoryErrorIsAbsorbed() throws IOException {
+    void heapExhaustionDuringMaterializationIsAbsorbed() throws IOException {
         Codec throwsOom = RedisStreamCodec.faultTolerant(new StubCodec((b, state) -> {
             throw new OutOfMemoryError("simulated");
         }));