Fix missing search aliases after reindex by deleting the concrete index atomically within the alias swap (#28667)

(cherry picked from commit 70cad10)

Author: mohityadav766

openmetadata-service/src/main/java/org/openmetadata/service/search/DefaultRecreateHandler.java

@@ -86,11 +86,8 @@ public ReindexContext reCreateIndexes(Set<String> entities) {
   public void finalizeReindex(EntityReindexContext context, boolean reindexSuccess) {
     String entityType = context.getEntityType();
     String canonicalIndex = context.getCanonicalIndex();
-    String activeIndex = context.getActiveIndex();
     String stagedIndex = context.getStagedIndex();
-    Set<String> existingAliases = context.getExistingAliases();
-    String canonicalAlias = context.getCanonicalAliases();
-    Set<String> parentAliases = context.getParentAliases();
+    Set<String> aliasesFromMapping = context.getExistingAliases();
 
     SearchRepository searchRepository = Entity.getSearchRepository();
     SearchClient searchClient = searchRepository.getSearchClient();
@@ -147,21 +144,20 @@ public void finalizeReindex(EntityReindexContext context, boolean reindexSuccess
       //                         is strictly worse than the writes going back to the canonical
       //                         alias target. Operators need to retry the reindex either way.
       try {
+        // The alias set was derived from indexMapping.json at recreate time via
+        // getAliasesFromMapping; finalize just attaches that captured set. Nothing is read from the
+        // live cluster, so the set is deterministic and matches the distributed promotion path.
         Set<String> aliasesToAttach = new HashSet<>();
-
-        existingAliases.stream()
-            .filter(alias -> alias != null && !alias.isBlank())
-            .forEach(aliasesToAttach::add);
-
-        if (!nullOrEmpty(canonicalAlias)) {
-          aliasesToAttach.add(canonicalAlias);
+        if (aliasesFromMapping != null) {
+          aliasesFromMapping.stream()
+              .filter(alias -> alias != null && !alias.isBlank())
+              .forEach(aliasesToAttach::add);
         }
 
-        parentAliases.stream()
-            .filter(alias -> alias != null && !alias.isBlank())
-            .forEach(aliasesToAttach::add);
-
-        aliasesToAttach.removeIf(alias -> alias == null || alias.isBlank());
+        if (aliasesToAttach.isEmpty()) {
+          abortPromotionWithoutAliases(entityType, stagedIndex);
+          return;
+        }
 
         Set<String> allEntityIndices = searchClient.listIndicesByPrefix(canonicalIndex);
         Set<String> oldIndicesToDelete = new HashSet<>();
@@ -171,44 +167,26 @@ public void finalizeReindex(EntityReindexContext context, boolean reindexSuccess
           }
         }
 
-        // After the first reindex, the canonical name is an alias on the previous staged, not a
-        // concrete index. OpenSearch's listIndicesByPrefix returns that alias name as one of its
-        // result keys, which then drives a delete-by-name attempt that fails with
-        // "matches an alias, specify the corresponding concrete indices" and burns ~31s of
-        // exponential backoff per entity (1+2+4+8+16s before giving up). With 60 entity types
-        // a full reindex wastes ~30 minutes in cleanup. Drop the alias name from the cleanup set
-        // when it is currently an alias — it does not need to be deleted; the swap moves the
-        // alias atomically and the underlying old concrete is in oldIndicesToDelete already.
-        if (!searchClient.getIndicesByAlias(canonicalIndex).isEmpty()) {
-          oldIndicesToDelete.remove(canonicalIndex);
-        }
+        Set<String> concreteToRemove =
+            resolveCanonicalRemoval(searchClient, canonicalIndex, oldIndicesToDelete);
 
         LOG.debug(
-            "finalizeReindex entity '{}': aliases={}, oldIndices={}, stagedIndex={}",
+            "finalizeReindex entity '{}': aliases={}, oldIndices={}, stagedIndex={}, "
+                + "concreteToRemove={}",
             entityType,
             aliasesToAttach,
             oldIndicesToDelete,
-            stagedIndex);
-
-        if (oldIndicesToDelete.contains(canonicalIndex)) {
-          if (searchClient.indexExists(canonicalIndex)) {
-            searchClient.deleteIndexWithBackoff(canonicalIndex);
-            oldIndicesToDelete.remove(canonicalIndex);
-            LOG.info("Cleaned up old index '{}' for entity '{}'.", canonicalIndex, entityType);
-          }
-        }
+            stagedIndex,
+            concreteToRemove);
 
-        if (!aliasesToAttach.isEmpty()) {
-          boolean swapSuccess =
-              searchClient.swapAliases(oldIndicesToDelete, stagedIndex, aliasesToAttach);
-          if (!swapSuccess) {
-            LOG.error(
-                "Failed to atomically swap aliases for entity '{}'. Old indices will not be deleted.",
-                entityType);
-            return;
-          }
-        } else {
-          LOG.warn("Entity '{}': aliasesToAttach is empty, skipping alias swap", entityType);
+        boolean swapSuccess =
+            searchClient.swapAliases(
+                oldIndicesToDelete, stagedIndex, aliasesToAttach, concreteToRemove);
+        if (!swapSuccess) {
+          LOG.error(
+              "Failed to atomically swap aliases for entity '{}'. Old indices will not be deleted.",
+              entityType);
+          return;
         }
 
         LOG.info(
@@ -344,6 +322,11 @@ public void promoteEntityIndex(EntityReindexContext context, boolean reindexSucc
       Set<String> aliasesToAttach =
           getAliasesFromMapping(indexMapping, searchRepository.getClusterAlias());
 
+      if (aliasesToAttach.isEmpty()) {
+        abortPromotionWithoutAliases(entityType, stagedIndex);
+        return;
+      }
+
       Set<String> allEntityIndices = searchClient.listIndicesByPrefix(canonicalIndex);
       Set<String> oldIndicesToDelete = new HashSet<>();
       for (String oldIndex : allEntityIndices) {
@@ -352,36 +335,29 @@ public void promoteEntityIndex(EntityReindexContext context, boolean reindexSucc
         }
       }
 
+      Set<String> concreteToRemove =
+          resolveCanonicalRemoval(searchClient, canonicalIndex, oldIndicesToDelete);
+
       LOG.debug(
-          "promoteEntityIndex '{}': aliases={}, oldIndices={}, stagedIndex={}",
+          "promoteEntityIndex '{}': aliases={}, oldIndices={}, stagedIndex={}, concreteToRemove={}",
           entityType,
           aliasesToAttach,
           oldIndicesToDelete,
-          stagedIndex);
-
-      if (oldIndicesToDelete.contains(canonicalIndex)) {
-        if (searchClient.indexExists(canonicalIndex)) {
-          searchClient.deleteIndexWithBackoff(canonicalIndex);
-          oldIndicesToDelete.remove(canonicalIndex);
-          LOG.info("Cleaned up old index '{}' for entity '{}'.", canonicalIndex, entityType);
-        }
-      }
+          stagedIndex,
+          concreteToRemove);
 
-      if (!aliasesToAttach.isEmpty()) {
-        boolean swapSuccess =
-            searchClient.swapAliases(oldIndicesToDelete, stagedIndex, aliasesToAttach);
-        if (!swapSuccess) {
-          LOG.error(
-              "Failed to atomically swap aliases for entity '{}'. "
-                  + "oldIndices={}, stagedIndex={}, aliases={}",
-              entityType,
-              oldIndicesToDelete,
-              stagedIndex,
-              aliasesToAttach);
-          return;
-        }
-      } else {
-        LOG.warn("Entity '{}': aliasesToAttach is empty, skipping alias swap", entityType);
+      boolean swapSuccess =
+          searchClient.swapAliases(
+              oldIndicesToDelete, stagedIndex, aliasesToAttach, concreteToRemove);
+      if (!swapSuccess) {
+        LOG.error(
+            "Failed to atomically swap aliases for entity '{}'. "
+                + "oldIndices={}, stagedIndex={}, aliases={}",
+            entityType,
+            oldIndicesToDelete,
+            stagedIndex,
+            aliasesToAttach);
+        return;
       }
 
       LOG.info(
@@ -419,6 +395,60 @@ public void promoteEntityIndex(EntityReindexContext context, boolean reindexSucc
     }
   }
 
+  /**
+   * Aborts promotion when no aliases resolved for the staged index. Critically this skips the
+   * old-index cleanup: deleting the previously serving index while no alias points at the staged
+   * index would orphan the canonical alias and surface to users as
+   * "Failed to find index openmetadata_*_search_index". The staged index is retained (its routing
+   * is cleared by the caller's finally block) so a subsequent reindex can recover.
+   */
+  private void abortPromotionWithoutAliases(String entityType, String stagedIndex) {
+    LOG.error(
+        "Entity '{}': no aliases resolved for staged index '{}'. Skipping alias swap and old-index "
+            + "cleanup to avoid orphaning the canonical alias. Staged index retained; reindex must "
+            + "be retried.",
+        entityType,
+        stagedIndex);
+    ReindexingMetrics metrics = ReindexingMetrics.getInstance();
+    if (metrics != null) {
+      metrics.recordPromotionFailure(entityType);
+    }
+  }
+
+  /**
+   * Resolves how the canonical index name participates in the alias swap and prunes it from {@code
+   * oldIndicesToDelete}.
+   *
+   * <ul>
+   *   <li><b>Concrete index sharing the alias name</b> (the first-install / post-orphan shape): it
+   *       is returned so the caller hands it to {@link SearchClient#swapAliases(Set, String, Set,
+   *       Set)} for an atomic {@code remove_index}. Deleting it in a separate step before the swap
+   *       opens a window where the index is gone but the alias is not yet attached — if the alias
+   *       add then fails (e.g. the delete has not propagated, so OS/ES still sees an index with the
+   *       alias name), the canonical name resolves to nothing and users hit "Failed to find index
+   *       openmetadata_*_search_index".
+   *   <li><b>Already an alias</b>: it is simply dropped from the delete set — the swap moves it
+   *       atomically, and a delete-by-name on an alias would fail ("matches an alias") and burn
+   *       retry backoff.
+   * </ul>
+   *
+   * @return the concrete indices (0 or 1) to remove atomically within the swap
+   */
+  private Set<String> resolveCanonicalRemoval(
+      SearchClient searchClient, String canonicalIndex, Set<String> oldIndicesToDelete) {
+    Set<String> concreteToRemove = new HashSet<>();
+    if (oldIndicesToDelete.contains(canonicalIndex)) {
+      boolean isConcreteIndex =
+          searchClient.getIndicesByAlias(canonicalIndex).isEmpty()
+              && searchClient.indexExists(canonicalIndex);
+      if (isConcreteIndex) {
+        concreteToRemove.add(canonicalIndex);
+      }
+      oldIndicesToDelete.remove(canonicalIndex);
+    }
+    return concreteToRemove;
+  }
+
   /**
    * Gets aliases from indexMapping.json configuration.
    */
@@ -501,18 +531,18 @@ protected void recreateIndexFromMapping(
     applyBulkBuildSettings(searchClient, stagedIndexName, entityType);
     searchRepository.registerStagedIndex(entityType, stagedIndexName);
 
-    Set<String> existingAliases =
-        activeIndexName != null ? searchClient.getAliases(activeIndexName) : new HashSet<>();
-
-    // Add the default index
-    existingAliases.add(indexMapping.getAlias(clusterAlias));
-    existingAliases.add(indexMapping.getIndexName(clusterAlias));
+    // Aliases to attach come solely from indexMapping.json (short alias + parent aliases + raw
+    // canonical index name) — never from whatever happens to be on the live index in ES. Reading
+    // existing aliases off the cluster is non-deterministic (it propagates stray/leftover aliases),
+    // adds a cluster round-trip and failure point, and diverges from the distributed promotion path
+    // (promoteEntityIndex). Both paths now funnel through the same getAliasesFromMapping helper.
+    Set<String> aliasesFromMapping = getAliasesFromMapping(indexMapping, clusterAlias);
     context.add(
         entityType,
         canonicalIndexName,
         activeIndexName,
         stagedIndexName,
-        existingAliases,
+        aliasesFromMapping,
         indexMapping.getAlias(clusterAlias),
         indexMapping.getParentAliases(clusterAlias));
 

openmetadata-service/src/main/java/org/openmetadata/service/search/IndexManagementClient.java

@@ -98,16 +98,30 @@ public interface IndexManagementClient {
   void removeAliases(String indexName, Set<String> aliases);
 
   /**
-   * Atomically swap aliases from old indices to a new index.
-   * This operation removes the specified aliases from any old indices and adds them to the new index
-   * in a single atomic operation, ensuring zero-downtime during index promotion.
+   * Atomically swap aliases from old indices to a new index, optionally deleting concrete indices
+   * in the same request. Removing the aliases from old indices, deleting any {@code indicesToRemove}
+   * (via {@code remove_index}) and adding the aliases to the new index all happen in a single atomic
+   * {@code updateAliases} call.
+   *
+   * <p>{@code indicesToRemove} is for the first-install / post-orphan shape where the canonical name
+   * (e.g. {@code table_search_index}) is still a <em>concrete</em> index sharing the alias name.
+   * Deleting it separately before the swap would orphan the alias if the alias add then failed;
+   * folding the delete into the atomic request means a failure is a no-op — the concrete index and
+   * its live aliases survive — instead of leaving the canonical name pointing at nothing.
    *
    * @param oldIndices the set of old index names to remove aliases from
    * @param newIndex the new index name to add aliases to
    * @param aliases the set of aliases to swap
+   * @param indicesToRemove concrete indices to delete atomically within the same request
    * @return true if the swap was successful, false otherwise
    */
-  boolean swapAliases(Set<String> oldIndices, String newIndex, Set<String> aliases);
+  boolean swapAliases(
+      Set<String> oldIndices, String newIndex, Set<String> aliases, Set<String> indicesToRemove);
+
+  /** Swap aliases without atomically removing any concrete index. */
+  default boolean swapAliases(Set<String> oldIndices, String newIndex, Set<String> aliases) {
+    return swapAliases(oldIndices, newIndex, aliases, Set.of());
+  }
 
   /**
    * Get all aliases for an index.

openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchClient.java

@@ -263,8 +263,9 @@ public void removeAliases(String indexName, Set<String> aliases) {
   }
 
   @Override
-  public boolean swapAliases(Set<String> oldIndices, String newIndex, Set<String> aliases) {
-    return indexManager.swapAliases(oldIndices, newIndex, aliases);
+  public boolean swapAliases(
+      Set<String> oldIndices, String newIndex, Set<String> aliases, Set<String> indicesToRemove) {
+    return indexManager.swapAliases(oldIndices, newIndex, aliases, indicesToRemove);
   }
 
   @Override

openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchIndexManager.java

@@ -330,35 +330,43 @@ public void removeAliases(String indexName, Set<String> aliases) {
   }
 
   @Override
-  public boolean swapAliases(Set<String> oldIndices, String newIndex, Set<String> aliases) {
+  public boolean swapAliases(
+      Set<String> oldIndices, String newIndex, Set<String> aliases, Set<String> indicesToRemove) {
     if (!isClientAvailable) {
       LOG.error("ElasticSearch client is not available. Cannot swap aliases.");
       return false;
     }
-    if (aliases == null || aliases.isEmpty()) {
-      LOG.debug("No aliases to swap for index {}", newIndex);
+    Set<String> finalAliases = aliases == null ? Set.of() : aliases;
+    Set<String> finalIndicesToRemove = indicesToRemove == null ? Set.of() : indicesToRemove;
+    if (finalAliases.isEmpty() && finalIndicesToRemove.isEmpty()) {
+      LOG.debug("No aliases to swap and no indices to remove for index {}", newIndex);
       return true;
     }
-    if (oldIndices == null) {
-      oldIndices = new HashSet<>();
-    }
-
-    Set<String> finalOldIndices = oldIndices;
+    Set<String> finalOldIndices = oldIndices == null ? new HashSet<>() : oldIndices;
     try {
       UpdateAliasesRequest request =
           UpdateAliasesRequest.of(
               updateBuilder -> {
                 // First, remove aliases from all old indices
                 for (String oldIndex : finalOldIndices) {
-                  for (String alias : aliases) {
+                  for (String alias : finalAliases) {
                     updateBuilder.actions(
                         actionBuilder ->
                             actionBuilder.remove(
                                 removeBuilder -> removeBuilder.index(oldIndex).alias(alias)));
                   }
                 }
-                // Then, add aliases to the new index
-                for (String alias : aliases) {
+                // Then delete any concrete index sharing the alias name, atomically, so the alias
+                // add below cannot race a separate delete and orphan the canonical name.
+                for (String indexToRemove : finalIndicesToRemove) {
+                  updateBuilder.actions(
+                      actionBuilder ->
+                          actionBuilder.removeIndex(
+                              removeIndexBuilder ->
+                                  removeIndexBuilder.index(indexToRemove).mustExist(false)));
+                }
+                // Finally, add aliases to the new index
+                for (String alias : finalAliases) {
                   updateBuilder.actions(
                       actionBuilder ->
                           actionBuilder.add(addBuilder -> addBuilder.index(newIndex).alias(alias)));
@@ -370,25 +378,18 @@ public boolean swapAliases(Set<String> oldIndices, String newIndex, Set<String>
 
       if (response.acknowledged()) {
         LOG.info(
-            "Atomically swapped aliases {} from indices {} to index {}",
-            aliases,
-            finalOldIndices,
-            newIndex);
+            "Atomically swapped aliases {} to index {} (removed indices {}, detached from {})",
+            finalAliases,
+            newIndex,
+            finalIndicesToRemove,
+            finalOldIndices);
         return true;
       } else {
-        LOG.warn(
-            "Alias swap from indices {} to index {} was not acknowledged",
-            finalOldIndices,
-            newIndex);
+        LOG.warn("Alias swap to index {} was not acknowledged", newIndex);
         return false;
       }
     } catch (Exception e) {
-      LOG.error(
-          "Failed to swap aliases {} from indices {} to index {}",
-          aliases,
-          finalOldIndices,
-          newIndex,
-          e);
+      LOG.error("Failed to swap aliases {} to index {}", finalAliases, newIndex, e);
       return false;
     }
   }