fix(search): make the OpenSearch write leg durable once it serves reads (#37276)

dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImpl.java

@@ -2426,16 +2426,32 @@ public void putToIndex(final IndexBulkRequest bulkRequest) {
             } catch (final Exception e) {
                 esException = new DotRuntimeException(e.getMessage(), e);
             }
+            RuntimeException osException = null;
             try {
                 operationsOS.putToIndex(dual.osReq);
             } catch (final Exception e) {
-                Logger.warnAndDebug(this.getClass(),
-                        "OS shadow write failed in putToIndex — "
-                                + "OS index may diverge until next reindex. Cause: " + e.getMessage(), e);
+                if (isReadEnabled()) {
+                    // Phase 2: OS serves reads (PhaseRouter.readProvider), so a failure here is
+                    // not a shadow divergence — it leaves the index users actually query out of
+                    // sync with the database (#37276). Surface it once ES has been given its
+                    // chance, so the caller can retry rather than assume the write landed.
+                    osException = (e instanceof RuntimeException)
+                            ? (RuntimeException) e
+                            : new DotRuntimeException(e.getMessage(), e);
+                } else {
+                    Logger.warnAndDebug(this.getClass(),
+                            "OS shadow write failed in putToIndex — "
+                                    + "OS index may diverge until next reindex. Cause: " + e.getMessage(), e);
+                }
             }
+            // ES stays authoritative when both legs fail: its exception is the one callers have
+            // always seen, and demoting it would change behaviour beyond this fix.
             if (esException != null) {
                 throw esException;
             }
+            if (osException != null) {
+                throw osException;
+            }
         } else {
             // Single-provider phase (0 or 3): forward to the sole active provider.
             // Failures propagate normally — there is no secondary provider to fall back to.
@@ -2495,10 +2511,18 @@ public IndexBulkProcessor createBulkProcessor(final IndexBulkListener bulkListen
         final boolean isDualWrite = providers.size() > 1;
         final List<CompositeBulkProcessor.Entry> entries = new ArrayList<>();
         for (final ContentletIndexOperations ops : providers) {
-            // OS is the shadow index in Phases 1 and 2 (dual-write): it replicates ES writes
-            // but is not yet the source of truth. In Phase 3 isDualWrite=false, so shadow=false
-            // and OS becomes the primary — failures propagate normally from that point.
-            final boolean shadow = isDualWrite && ops == operationsOS;
+            // OS is the shadow index in Phase 1 only: it replicates ES writes and nothing reads
+            // from it, so a failure there is genuinely tolerable (ADR-0009).
+            //
+            // Phase 2 is different and used to be handled as if it were Phase 1. Reads are served
+            // by OS from Phase 2 onwards (PhaseRouter.readProvider), so an OS write failure is
+            // immediately user-visible: a removal lost on the OS leg leaves an orphaned document
+            // in the very index being queried — the #37276 symptom, in the phase the migration
+            // spends the longest in. Treating OS as a shadow there also meant the journal entry
+            // was acked on the ES result alone and never retried.
+            //
+            // In Phase 3 isDualWrite=false, so shadow=false and OS is simply the primary.
+            final boolean shadow = isDualWrite && ops == operationsOS && !isReadEnabled();
             // Each provider gets its own listener so counters and log output stay per-provider.
             // The shadow OS listener never touches the reindex queue or triggers a rebuild.
             final IndexBulkListener listenerForOps = shadow

dotCMS/src/main/java/com/dotcms/content/index/opensearch/ContentletIndexOperationsOS.java

@@ -337,7 +337,12 @@ void handleBulkResponse(final BulkResponse response) {
             }
         }
 
-        throw new DotRuntimeException(detail.toString());
+        // errors() is expected to imply at least one item carrying an error, but if that ever
+        // stops holding we must not hand the caller a blank exception — a failure with no
+        // message is barely better than the silent return this replaced.
+        throw new DotRuntimeException(detail.length() > 0
+                ? detail.toString()
+                : "OS bulk reported errors but no item carried an error cause");
     }
 
     // =========================================================================

dotCMS/src/test/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImplPhase2ReadDurabilityTest.java

@@ -0,0 +1,141 @@
+package com.dotcms.content.elasticsearch.business;
+
+import static com.dotcms.content.index.IndexConfigHelper.MigrationPhase.FLAG_KEY;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+import com.dotcms.content.elasticsearch.business.ContentletIndexAPIImpl.DualIndexBulkRequest;
+import com.dotcms.content.elasticsearch.business.ContentletIndexAPIImplPhaseTest.FakeContentletIndexOperations;
+import com.dotcms.content.elasticsearch.business.ContentletIndexAPIImplPhaseTest.FakeIndexAPI;
+import com.dotcms.content.elasticsearch.business.ContentletIndexAPIImplPhaseTest.FakeIndiciesAPI;
+import com.dotcms.content.elasticsearch.business.ContentletIndexAPIImplPhaseTest.FakeVersionedIndicesAPI;
+import com.dotcms.content.index.domain.IndexBulkRequest;
+import com.dotmarketing.exception.DotRuntimeException;
+import com.dotmarketing.util.Config;
+import java.util.List;
+import org.junit.After;
+import org.junit.Test;
+
+/**
+ * Unit tests for the durability of the OpenSearch write leg once OpenSearch serves reads.
+ *
+ * <p>Covers the Phase 2 gap found while reviewing
+ * <a href="https://github.com/dotCMS/core/pull/37320">#37320</a> for
+ * <a href="https://github.com/dotCMS/core/issues/37276">#37276</a>.</p>
+ *
+ * <h2>The gap</h2>
+ * <p>ADR-0009 says a failed write to the shadow store is logged and must not impact operations.
+ * That is right for <b>Phase 1</b>, where nothing reads from OpenSearch. From <b>Phase 2</b>
+ * onwards {@code PhaseRouter#readProvider} serves reads from OpenSearch while writes still
+ * fan out ES-primary / OS-shadow — so a removal lost on the OS leg left an orphaned document in
+ * the very index being queried. That is the original defect, in the phase the migration spends
+ * the longest in.</p>
+ *
+ * <p>The fix is scoped by <em>who serves reads</em>, not by dual-write: OS stays fire-and-forget
+ * while it is invisible, and becomes durable the moment it is readable. ADR-0009's intent is
+ * preserved; only its Phase 2 assumption is corrected.</p>
+ */
+public class ContentletIndexAPIImplPhase2ReadDurabilityTest {
+
+    @After
+    public void clearPhase() {
+        Config.setProperty(FLAG_KEY, null);
+    }
+
+    private static void setPhase(final int ordinal) {
+        Config.setProperty(FLAG_KEY, String.valueOf(ordinal));
+    }
+
+    /** A bulk request handle with no behaviour — only identity matters here. */
+    private static final class StubBulkRequest implements IndexBulkRequest {
+        @Override
+        public int size() {
+            return 1;
+        }
+    }
+
+    /** Provider whose {@code putToIndex} either fails or records the call. */
+    private static final class RecordingOperations extends FakeContentletIndexOperations {
+
+        private final boolean fail;
+        int putCalls = 0;
+
+        RecordingOperations(final boolean fail) {
+            this.fail = fail;
+        }
+
+        @Override
+        public void putToIndex(final IndexBulkRequest req) {
+            putCalls++;
+            if (fail) {
+                throw new DotRuntimeException("bulk write rejected");
+            }
+        }
+    }
+
+    private static ContentletIndexAPIImpl buildApi(final RecordingOperations es,
+            final RecordingOperations os) {
+        return new ContentletIndexAPIImpl(es, os,
+                new FakeIndexAPI(List.of()), new FakeIndiciesAPI(), new FakeVersionedIndicesAPI());
+    }
+
+    /**
+     * Given Scenario: Phase 1 — ES is primary and nothing reads from OpenSearch. The OS leg fails.
+     * When : putToIndex fans the batch out to both providers.
+     * Then : the caller is not told. ADR-0009's fire-and-forget shadow policy is preserved where
+     *        it belongs, and this test is what stops the Phase 2 fix from over-reaching into it.
+     */
+    @Test
+    public void test_phase1_shadowFailure_isStillSwallowed() {
+        setPhase(1);
+        final RecordingOperations es = new RecordingOperations(false);
+        final RecordingOperations os = new RecordingOperations(true);
+
+        buildApi(es, os).putToIndex(
+                new DualIndexBulkRequest(new StubBulkRequest(), new StubBulkRequest()));
+
+        assertEquals("Both legs must still be attempted", 1, es.putCalls);
+        assertEquals(1, os.putCalls);
+    }
+
+    /**
+     * Given Scenario: Phase 2 — OpenSearch serves reads while writes are still dual. The OS leg
+     *                 fails.
+     * When : putToIndex fans the batch out to both providers.
+     * Then : the caller IS told, because the failure left the index that answers queries out of
+     *        sync with the database. Swallowing it here is what reproduced #37276 in Phase 2.
+     */
+    @Test
+    public void test_phase2_osFailure_reachesCaller_becauseOsServesReads() {
+        setPhase(2);
+        final RecordingOperations es = new RecordingOperations(false);
+        final RecordingOperations os = new RecordingOperations(true);
+        final ContentletIndexAPIImpl api = buildApi(es, os);
+
+        assertThrows(RuntimeException.class, () -> api.putToIndex(
+                new DualIndexBulkRequest(new StubBulkRequest(), new StubBulkRequest())));
+
+        assertEquals("ES must still have been written before the OS failure surfaces",
+                1, es.putCalls);
+    }
+
+    /**
+     * Given Scenario: Phase 2, and BOTH legs fail.
+     * When : putToIndex runs.
+     * Then : the ES exception is the one raised. ES stays authoritative for the caller's error;
+     *        demoting it would change behaviour beyond the gap being closed here.
+     */
+    @Test
+    public void test_phase2_bothFail_esExceptionWins() {
+        setPhase(2);
+        final RecordingOperations es = new RecordingOperations(true);
+        final RecordingOperations os = new RecordingOperations(true);
+        final ContentletIndexAPIImpl api = buildApi(es, os);
+
+        assertThrows(RuntimeException.class, () -> api.putToIndex(
+                new DualIndexBulkRequest(new StubBulkRequest(), new StubBulkRequest())));
+
+        assertEquals("The OS leg is always attempted, even when ES already failed",
+                1, os.putCalls);
+    }
+}

specs/37276-silent-index-delete-loss/contracts/putToIndex-failure-contract.md

@@ -30,16 +30,27 @@ Unchanged:
 
 ## Behavior per migration phase
 
-The router already isolates the shadow provider, so the new failure surfaces exactly where
-ADR-0009 requires and nowhere else. **The escalation is implemented in the providers, not in
-the router** — putting it in the router would break the phase 1–2 guarantee below.
+**The escalation is implemented in the providers, not in the router.** The router decides
+whether a provider's failure is tolerable; the providers only decide whether a bulk response
+counts as a failure at all.
 
 | Phase | Providers | Partial failure in ES | Partial failure in OS |
 |-------|-----------|-----------------------|-----------------------|
 | 0 | ES only | propagates | n/a |
-| 1, 2 | ES primary, OS shadow | propagates | logged, swallowed by the router (`ContentletIndexAPIImpl:2429-2435`) — **ADR-0009** |
+| 1 | ES primary, OS shadow | propagates | logged, swallowed — **ADR-0009** |
+| 2 | ES primary, OS shadow **but OS serves reads** | propagates | **propagates** |
 | 3 | OS only | n/a | propagates |
 
+Phase 2 is the row that is easy to get wrong. `PhaseRouter#readProvider` serves reads from
+OpenSearch from Phase 2 onwards, so an OS write failure there is not a shadow divergence — it
+leaves the index that answers queries out of sync with the database, which is the defect
+#37276 is about. The shadow treatment is therefore scoped by **who serves reads**
+(`isReadEnabled()`), not by whether the phase is dual-write. ADR-0009's intent is preserved:
+a store nobody reads from still cannot break a user operation.
+
+When both legs fail, the ES exception is the one raised — that is what callers have always
+seen.
+
 ## Impact on callers
 
 In-tree callers of the router method, all in `ContentletIndexAPIImpl`:

specs/37276-silent-index-delete-loss/release-note.md

@@ -24,6 +24,16 @@ of the write rather than as unexplained index drift weeks later. Treat a new `pu
 as a pre-existing condition now made visible — most often index write-queue saturation — and
 investigate the index cluster, not this release.
 
+**During the ES→OpenSearch migration, Phase 2 writes can now fail where they previously did
+not.** From Phase 2 onwards OpenSearch serves reads while writes still go to both clusters. A
+failed OpenSearch write used to be logged and ignored as a shadow divergence — which left
+orphaned documents in the very index being queried. It now reaches the caller and, on the
+reindex-journal path, marks the entry for retry.
+
+This affects every write in Phase 2, not only removals. An environment whose OpenSearch cluster
+is unhealthy will begin surfacing errors that were previously absorbed. Phase 1 is unchanged:
+while nothing reads from OpenSearch, a failed shadow write is still logged and ignored.
+
 **Log wording changed.** Bulk failures previously read `Error reindexing` regardless of the
 operation, including removals. They now read `Error applying (N) index operation(s)`. Any log
 alerting or saved search matching the old string needs updating.