fix(opensearch): stop Phase 3 reindex from orphaning ES rows in indicies table (#36077) (#36191)

fabrizzio-dotCMS · claude · web-flow · commit b4216cb4b1c8 · 2026-06-19T16:35:11.000Z
## Proposed Changes Fixes #36077. During OpenSearch migration **Phase 3** (OS primary, ES decommissioned), a full reindex left stale ES-side rows in the `indicies` table — the old `live`/`working` pair plus the transient `reindex_live`/`reindex_working` pair (all NULL version) — alongside the correctly-promoted `.os` (v3.X) pair. The table accumulated **6 rows instead of 2** on every Phase-3 reindex. ### Root cause An asymmetry in `ContentletIndexAPIImpl`: - `initAndPointReindex` wrote the legacy ES store **unconditionally** (`pointES` had no phase gate), so a Phase-3 reindex still inserted ES `reindex_*` rows and re-persisted the old ES `live`/`working` rows. - The Phase-3 switchover (`fullReindexSwitchoverOS`) only touches the OS store (`versionedIndicesAPI`) — it never promotes or clears those ES rows. So they orphaned. ### The fix (DB-only — never contacts the ES cluster, which may be down in Phase 3) 1. **Gate `pointES(...)`** in `initAndPointReindex` on `!isMigrationComplete()` so a Phase-3 reindex no longer writes ES pointers (mirrors the existing `isMigrationStarted()` gate on the OS write). 2. **Add `VersionedIndicesAPI.removeLegacyContentIndices()`** (delegating to `IndicesFactory`): deletes the NULL-version `live`/`working`/`reindex_live`/`reindex_working` rows, **preserves** the unmigrated `site_search` pointer and all OS (non-NULL version) rows, and flushes the index caches. Invoked best-effort by `fullReindexSwitchoverOS` after promoting OS (a housekeeping failure must not undo a successful switchover). **Physical index deletion is intentionally out of scope** — in Phase 3 ES may not be running, so touching the cluster is fragile. Orphaned physical indices are cleaned by the existing scheduled `DeleteInactiveLiveWorkingIndicesJob`. ### Why an explicit cleanup — and why earlier phases never needed one The legacy ES `reindex_*` rows were never purged by a dedicated call. In the classic ES path (phases 0–2) the cleanup is an **emergent property of `IndiciesFactory.point()`**: `fullReindexSwitchover()` rebuilds the entire `indicies` table from a *fresh* `IndiciesInfo.Builder` that sets only `working`/`live`/`site_search` and leaves `reindex_*` null, so `point()`'s delete-then-conditional-insert drops the reindex rows for free — and overwrites the old `working`/`live` rows in the same pass. The Phase-3 switchover (`fullReindexSwitchoverOS`) does **not** go through that full-table rebuild — it writes only the OS/versioned rows via `versionedIndicesAPI`, so the implicit cleanup that every earlier phase gets for free simply never runs and the legacy ES rows orphan. `removeLegacyContentIndices()` is the explicit equivalent of that implicit `point()` sweep, scoped to the OS-only path. It is therefore *restoring symmetry of outcome* (a clean 2-row table) across two structurally different switchover paths — not introducing a new, riskier deletion. **Scope guard:** the delete is intentionally narrow (`index_version IS NULL` + content-index types only), so it can never touch OS (non-NULL version) rows nor the still-unmigrated `site_search` pointer. The phase protection currently lives at the *call site* (`removeLegacyContentIndices()` is only invoked from `fullReindexSwitchoverOS`, which runs only in Phase 3); the method itself has no internal phase guard. ## Acceptance criteria - [x] After a Phase-3 reindex, no leftover ES `live`/`working` (NULL version) rows. - [x] Transient `reindex_live`/`reindex_working` rows removed. - [ ] Physical indices deleted — deferred to `DeleteInactiveLiveWorkingIndicesJob` (see scope note above). - [x] Re-running reindex in Phase 3 does not accumulate orphan rows. - [x] Integration coverage asserts the final `indicies` table state. ## Testing - `ContentletIndexAPIImplMigrationIntegrationTest#test_phase3_removeLegacyContentIndices_purgesEsRowsPreservesSiteSearchAndOs` — seeds the issue's exact 6-row orphan state and asserts the legacy ES content rows are purged while `site_search` and OS rows survive. - `ContentletIndexAPIImplPhaseTest` — fake updated for the new API method. - `./mvnw test-compile -pl :dotcms-core,:dotcms-integration` passes on JDK 25. > Note: the new IT still needs a run against a live OpenSearch Phase-3 environment (`opensearch-upgrade` profile) — a full-reindex E2E was deliberately avoided due to the known `inferIndexToHit` gap (#36054); the test validates the cleanup guarantee deterministically at the DB level. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImpl.java
@@ -1092,10 +1092,14 @@ private void initAndPointReindex(final String ts) throws DotDataException {
                     "Error creating reindex indices for ts=" + ts + ": " + e.getMessage(), e);
         }
         // ── LEGACY ES — remove after Phase 3 migration ────────────────────────
-        // Persist reindex slots to the legacy ES store, preserving existing working/live pointers
-        pointES(null, null,
-                operationsES.toPhysicalName(reindexWorkingName),
-                operationsES.toPhysicalName(reindexLiveName));
+        // Persist reindex slots to the legacy ES store, preserving existing working/live pointers.
+        // Skipped in Phase 3: ES is decommissioned, so writing ES reindex pointers here would only
+        // create orphan NULL-version rows that the OS-only switchover never cleans up (#36077).
+        if (!isMigrationComplete()) {
+            pointES(null, null,
+                    operationsES.toPhysicalName(reindexWorkingName),
+                    operationsES.toPhysicalName(reindexLiveName));
+        }
         // ── END LEGACY ES ─────────────────────────────────────────────────────
 
         // Persist OS reindex slots, preserving existing working/live pointers.
@@ -1221,33 +1225,38 @@ public synchronized boolean fullReindexSwitchover(Connection conn, final boolean
             // ── OS mirror (Phases 1 and 2, best-effort) ──────────────────────
             // The OS shadow index has its own reindex slots that must be promoted
             // in lock-step with ES. A failure here must never abort the ES result.
+            List<String> newActiveOs = List.of();
             if (isMigrationStarted()) {
                 try {
                     final Optional<VersionedIndices> osExisting =
                             versionedIndicesAPI.loadDefaultVersionedIndices();
+                    final Optional<String> osWorking =
+                            osExisting.flatMap(VersionedIndices::reindexWorking);
+                    final Optional<String> osLive =
+                            osExisting.flatMap(VersionedIndices::reindexLive);
                     final VersionedIndicesImpl.Builder osBuilder = VersionedIndicesImpl.builder();
                     // Promote OS reindex slots → active; omitting reindexWorking/reindexLive
                     // from the builder clears them to Optional.empty() in the store.
-                    osExisting.flatMap(VersionedIndices::reindexWorking).ifPresent(osBuilder::working);
-                    osExisting.flatMap(VersionedIndices::reindexLive).ifPresent(osBuilder::live);
+                    osWorking.ifPresent(osBuilder::working);
+                    osLive.ifPresent(osBuilder::live);
                     versionedIndicesAPI.saveIndices(osBuilder.build());
+                    // Capture the promoted OS names (.os-tagged, from the OS store) so they can be
+                    // optimized on the OS provider directly — see optimizeNewActiveIndicesAsync.
+                    if (osWorking.isPresent() && osLive.isPresent()) {
+                        newActiveOs = List.of(osWorking.get(), osLive.get());
+                    }
                 } catch (Exception osEx) {
                     Logger.warn(this, "Could not mirror reindex switchover to OS store", osEx);
                 }
             }
 
-            // Async: merge ES index segments and expand replicas on the newly active indices.
-            // Uses newInfo (ES reindex → active names), not oldInfo (old active names).
-            final List<String> newActiveEs = List.of(newInfo.getWorking(), newInfo.getLive());
-            DotConcurrentFactory.getInstance().getSubmitter().submit(() -> {
-                try {
-                    Logger.info(this.getClass(), "Updating and optimizing ElasticSearch Indexes");
-                    optimize(newActiveEs);
-                } catch (Exception e) {
-                    Logger.warnAndDebug(this.getClass(),
-                            "unable to expand ES replicas:" + e.getMessage(), e);
-                }
-            });
+            // Async: merge index segments and expand replicas on the newly active indices,
+            // optimizing EACH provider with the names it actually holds — ES bare names from
+            // newInfo, OS .os-tagged names from the store. Routing through the phase-aware
+            // optimize() would send the ES names to the OS read provider in Phase 2 and hit
+            // index_not_found (then fall back to ES), which is noisy and skips the OS optimize.
+            optimizeNewActiveIndicesAsync(
+                    List.of(newInfo.getWorking(), newInfo.getLive()), newActiveOs);
 
             notifyAdminsOfFailedReindex();
 
@@ -1314,24 +1323,70 @@ private boolean fullReindexSwitchoverOS(final boolean forceSwitch) throws Except
         existing.reindexLive().ifPresent(osBuilder::live);
         versionedIndicesAPI.saveIndices(osBuilder.build());
 
+        // Purge leftover legacy ES content-index rows (NULL version) from the indicies table:
+        // old live/working plus any transient reindex_live/reindex_working that predate Phase 3.
+        // ES is decommissioned, so these rows are pure orphans — without this the table would
+        // accumulate stale ES rows on every Phase-3 reindex (#36077). DB-only: never contacts the
+        // ES cluster (which may be down). Best-effort — the OS promotion above already succeeded
+        // and must not be undone by a housekeeping failure. Physical ES index deletion is left to
+        // the scheduled DeleteInactiveLiveWorkingIndicesJob.
+        try {
+            versionedIndicesAPI.removeLegacyIndices();
+        } catch (Exception cleanupEx) {
+            Logger.warn(this, "Phase 3 switchover: could not purge legacy ES indicies rows", cleanupEx);
+        }
+
         // Async: optimize the newly active OS indices (merge segments, adjust replicas).
         // reindexWorking/reindexLive presence was validated at the top of this method.
+        // ES is decommissioned in Phase 3, so only the OS provider is optimized.
         final String newWorking = existing.reindexWorking().orElseThrow();
         final String newLive    = existing.reindexLive().orElseThrow();
-        DotConcurrentFactory.getInstance().getSubmitter().submit(() -> {
-            try {
-                Logger.info(this.getClass(), "Updating and optimizing OpenSearch Indexes");
-                optimize(List.of(newWorking, newLive));
-            } catch (Exception e) {
-                Logger.warnAndDebug(this.getClass(),
-                        "unable to optimize OS indices:" + e.getMessage(), e);
-            }
-        });
+        optimizeNewActiveIndicesAsync(List.of(), List.of(newWorking, newLive));
 
         notifyAdminsOfFailedReindex();
         return true;
     }
 
+    /**
+     * Optimizes (force-merges) the newly-promoted indices after a reindex switchover, targeting
+     * each provider with the names it actually holds: ES with its bare names, OS with its
+     * {@code .os}-tagged names. Both calls go to the vendor implementation directly
+     * ({@code operationsES/OS.indexAPI()}) rather than the phase-aware {@code optimize()} router,
+     * because the router routes optimize through the read provider — in Phase 2 that is OS, so the
+     * ES names would be sent to OS, miss (the OS index carries the {@code .os} tag), and only
+     * succeed via the ES fallback while logging a misleading error and skipping the OS optimize.
+     *
+     * <p>Runs asynchronously and is best-effort per provider: a force-merge failure (including an
+     * OS shadow failure in dual-write phases) is logged and swallowed and never affects the
+     * completed switchover.</p>
+     *
+     * @param esNames ES physical (bare) names to optimize, or empty to skip ES (e.g. Phase 3)
+     * @param osNames OS physical ({@code .os}-tagged) names to optimize, or empty to skip OS
+     */
+    private void optimizeNewActiveIndicesAsync(final List<String> esNames,
+            final List<String> osNames) {
+        DotConcurrentFactory.getInstance().getSubmitter().submit(() -> {
+            if (esNames != null && !esNames.isEmpty()) {
+                try {
+                    Logger.info(this.getClass(), "Updating and optimizing ElasticSearch Indexes");
+                    operationsES.indexAPI().optimize(esNames);
+                } catch (Exception e) {
+                    Logger.warnAndDebug(this.getClass(),
+                            "unable to optimize ES indices:" + e.getMessage(), e);
+                }
+            }
+            if (osNames != null && !osNames.isEmpty()) {
+                try {
+                    Logger.info(this.getClass(), "Updating and optimizing OpenSearch Indexes");
+                    operationsOS.indexAPI().optimize(osNames);
+                } catch (Exception e) {
+                    Logger.warnAndDebug(this.getClass(),
+                            "unable to optimize OS indices:" + e.getMessage(), e);
+                }
+            }
+        });
+    }
+
     /**
      * Sends a system-wide warning message to CMS administrators if any content documents
      * failed to be reindexed during the last full-reindex run.
diff --git a/dotCMS/src/main/java/com/dotcms/content/index/IndexAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/index/IndexAPIImpl.java
@@ -131,7 +131,29 @@ public OSIndexAPIImpl osImpl() {
 
     @Override
     public boolean optimize(final List<String> indexNames) {
-        return router.read(impl -> impl.optimize(indexNames));
+        // Tag-dispatch: each index name declares its owning provider via its vendor tag
+        // (OS names carry the .os suffix; untagged names are legacy ES). The list handed in
+        // comes from getIndices(), which in dual-write phases aggregates BOTH providers, so a
+        // mixed list (ES bare names + OS .os-tagged names) routed to a single provider via
+        // router.read() would hit index_not_found_exception on the foreign-tagged names —
+        // the exact Phase 2 optimize misrouting reported against the /optimize endpoint.
+        // Force-merge each provider only with the names it actually holds; skip a provider
+        // when its subset is empty so Phase 0 never contacts OS and Phase 3 never contacts ES.
+        if (indexNames == null || indexNames.isEmpty()) {
+            return true;
+        }
+        final Map<IndexTag, List<String>> byVendor = indexNames.stream()
+                .collect(Collectors.groupingBy(IndexTag::resolve));
+        boolean result = true;
+        final List<String> esNames = byVendor.getOrDefault(IndexTag.ES, List.of());
+        if (!esNames.isEmpty()) {
+            result &= router.esImpl().optimize(esNames);
+        }
+        final List<String> osNames = byVendor.getOrDefault(IndexTag.OS, List.of());
+        if (!osNames.isEmpty()) {
+            result &= router.osImpl().optimize(osNames);
+        }
+        return result;
     }
 
     @Override
diff --git a/dotCMS/src/main/java/com/dotcms/content/index/IndicesFactory.java b/dotCMS/src/main/java/com/dotcms/content/index/IndicesFactory.java
@@ -132,6 +132,21 @@ public interface IndicesFactory {
     void insertIndexIfPresent(String indexName, String indexType,
             String version) throws DotDataException;
 
+    /**
+     * Removes the legacy ElasticSearch content-index pointers (NULL {@code index_version})
+     * from the {@code indicies} table: the {@code live} / {@code working} pair and the transient
+     * {@code reindex_live} / {@code reindex_working} pair.
+     *
+     * <p>Intended for Phase 3 (OpenSearch-only) cleanup, where ES is decommissioned and these
+     * NULL-version rows are pure orphans. The {@code site_search} pointer — also stored
+     * NULL-versioned but NOT part of the content-index migration — is deliberately preserved.
+     * OS rows (which carry a non-NULL {@code index_version}) are never touched.</p>
+     *
+     * @return the number of rows removed
+     * @throws DotDataException if a SQL error occurs
+     */
+    int removeLegacyIndices() throws DotDataException;
+
     /**
      * Validates that every present index name in {@code indicesInfo} carries the
      * {@link IndexTag#OS} marker ({@code .os} suffix) required for DB storage.
diff --git a/dotCMS/src/main/java/com/dotcms/content/index/IndicesFactoryImpl.java b/dotCMS/src/main/java/com/dotcms/content/index/IndicesFactoryImpl.java
@@ -43,6 +43,11 @@ public class IndicesFactoryImpl implements IndicesFactory {
         "SELECT COUNT(*) as count FROM indicies WHERE index_version = ? AND index_version IS NOT NULL";
     private static final String COUNT_INDICES_BY_VERSION_SQL =
         "SELECT COUNT(*) as count FROM indicies WHERE index_version = ? AND index_version IS NOT NULL";
+    // Phase 3 cleanup: remove the legacy ES content-index pointers (NULL version) while
+    // preserving the still-unmigrated site_search pointer. OS rows carry a non-NULL version.
+    private static final String DELETE_LEGACY_CONTENT_INDICES_SQL =
+        "DELETE FROM indicies WHERE index_version IS NULL " +
+        "AND index_type IN ('live', 'working', 'reindex_live', 'reindex_working')";
 
     @Override
     public Optional<VersionedIndices> loadIndices(String version) throws DotDataException {
@@ -168,6 +173,20 @@ public void removeVersion(String version) throws DotDataException {
         }
     }
 
+    @Override
+    public int removeLegacyIndices() throws DotDataException {
+        try {
+            final DotConnect dotConnect = new DotConnect();
+            final int deletedRows = dotConnect.executeUpdate(DELETE_LEGACY_CONTENT_INDICES_SQL);
+            Logger.info(this, "Removed " + deletedRows
+                    + " legacy ES content-index row(s) from indicies (Phase 3 cleanup)");
+            return deletedRows;
+        } catch (Exception e) {
+            Logger.error(this, "Failed to remove legacy ES content-index rows", e);
+            throw new DotDataException("Failed to remove legacy ES content-index rows", e);
+        }
+    }
+
     @Override
     public boolean versionExists(String version) throws DotDataException {
         if (!UtilMethods.isSet(version)) {
diff --git a/dotCMS/src/main/java/com/dotcms/content/index/VersionedIndicesAPI.java b/dotCMS/src/main/java/com/dotcms/content/index/VersionedIndicesAPI.java
@@ -104,6 +104,21 @@ public interface VersionedIndicesAPI {
      */
     Optional<VersionedIndices> loadDefaultVersionedIndices() throws DotDataException;
 
+    /**
+     * Removes the legacy ElasticSearch content-index pointers (NULL {@code index_version}) from
+     * the {@code indicies} table — the {@code live} / {@code working} and transient
+     * {@code reindex_live} / {@code reindex_working} rows — and flushes the index caches.
+     *
+     * <p>Intended for Phase 3 (OpenSearch-only) reindex cleanup: ES is decommissioned and these
+     * rows are orphans. The unmigrated {@code site_search} pointer and all OS (non-NULL version)
+     * rows are preserved. This is a DB-only operation — it never contacts the ES cluster, which
+     * may not be running in Phase 3.</p>
+     *
+     * @return the number of rows removed
+     * @throws DotDataException if a SQL error occurs
+     */
+    int removeLegacyIndices() throws DotDataException;
+
     /**
      * Clears all cached indices data.
      * This should be called when indices are modified outside of this API
diff --git a/dotCMS/src/main/java/com/dotcms/content/index/VersionedIndicesAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/index/VersionedIndicesAPIImpl.java
@@ -228,6 +228,33 @@ public Optional<VersionedIndices> loadDefaultVersionedIndices() throws DotDataEx
         return loadIndices(VersionedIndices.OPENSEARCH_3X);
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    @WrapInTransaction
+    @Override
+    public int removeLegacyIndices() throws DotDataException {
+        // Defense in depth: this purges the legacy ES content-index pointers (NULL version) and is
+        // only safe once ES is decommissioned (Phase 3). The sole caller (fullReindexSwitchoverOS)
+        // already runs only in Phase 3, but guard here too so a stray call in an earlier phase can
+        // never delete the still-active ES live/working rows.
+        if (!IndexConfigHelper.isMigrationComplete()) {
+            Logger.warn(this, "removeLegacyIndices() called outside Phase 3 "
+                    + "(PHASE_3_OPENSEARCH_ONLY); skipping — the legacy ES live/working rows are "
+                    + "still active in this phase.");
+            return 0;
+        }
+        final int removed = indicesFactory.removeLegacyIndices();
+        // Flush all index-related caches so no stale legacy names survive the deletion:
+        // 1. VersionedIndicesCache — our own versioned-index cache
+        cache.clearCache();
+        // 2. IndiciesCache — legacy (ES, non-versioned) index cache used by IndiciesFactory
+        CacheLocator.getIndiciesCache().clearCache();
+        // 3. ESQueryCache — cached search queries that may reference a deleted index name
+        CacheLocator.getESQueryCache().clearCache();
+        return removed;
+    }
+
     /**
      * {@inheritDoc}
      */
diff --git a/dotCMS/src/test/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImplPhaseTest.java b/dotCMS/src/test/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImplPhaseTest.java
@@ -576,12 +576,19 @@ public void point(final IndiciesInfo newInfo) {
     static class FakeVersionedIndicesAPI implements VersionedIndicesAPI {
 
         VersionedIndices stored = null;
+        int removeLegacyIndicesCalls = 0;
 
         @Override
         public Optional<VersionedIndices> loadDefaultVersionedIndices() {
             return Optional.ofNullable(stored);
         }
 
+        @Override
+        public int removeLegacyIndices() {
+            removeLegacyIndicesCalls++;
+            return 0;
+        }
+
         @Override
         public void saveIndices(final VersionedIndices info) {
             this.stored = info;
diff --git a/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImplMigrationIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImplMigrationIntegrationTest.java
