Skip to content

Commit 3f1f418

Browse files
mohityadav766claude
andcommitted
fix(search): drop must_exist on remove_index so OpenSearch alias swap succeeds (#28700)
PR #28667 added an atomic alias swap that folds the canonical concrete-index delete into the _aliases request via a remove_index action. The action was built as removeIndex(index).mustExist(false). OpenSearch's _aliases parser does not accept must_exist on remove_index and rejects the whole request with "[remove_index] unknown field [must_exist]" -> "[aliases] failed to parse field [actions]" (HTTP 400), so the alias add in the same body never applies. Elasticsearch tolerates the field, which is why it passed review. On a fresh install every canonical *_search_index is a concrete index, so the remove_index action fires for all entities and every swap fails -> no canonical aliases are attached -> the canonical name resolves to nothing. This surfaced as the AI Platform CAIP integration test failing with "table_search_index missing embedding field". Drop must_exist from the remove_index action in both OpenSearchIndexManager and ElasticSearchIndexManager. It is unnecessary: resolveCanonicalRemoval only forwards indices already confirmed to exist via indexExists(). Both engine paths are now identical. Add AliasSwapConcreteRemovalIT, which exercises the first-install shape (concrete canonical in indicesToRemove) against the live cluster: it fails before this fix on OpenSearch and passes after. The existing DefaultRecreateHandlerTest mocks swapAliases, so it never exercised the request body where this bug lives. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit d800ba4)
1 parent dce2cf9 commit 3f1f418

3 files changed

Lines changed: 114 additions & 4 deletions

File tree

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright 2026 Collate
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
* Unless required by applicable law or agreed to in writing, software
8+
* distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
* See the License for the specific language governing permissions and
11+
* limitations under the License.
12+
*/
13+
package org.openmetadata.it.tests;
14+
15+
import static org.junit.jupiter.api.Assertions.assertEquals;
16+
import static org.junit.jupiter.api.Assertions.assertTrue;
17+
18+
import java.util.List;
19+
import java.util.Set;
20+
import org.junit.jupiter.api.AfterEach;
21+
import org.junit.jupiter.api.BeforeAll;
22+
import org.junit.jupiter.api.Test;
23+
import org.openmetadata.it.util.SdkClients;
24+
import org.openmetadata.service.Entity;
25+
import org.openmetadata.service.search.SearchClient;
26+
27+
/**
28+
* Reproduces the regression from PR #28667 where the atomic alias swap orphaned every canonical
29+
* {@code *_search_index} alias on OpenSearch.
30+
*
31+
* <p>On a fresh install the canonical name (e.g. {@code table_search_index}) is a <em>concrete</em>
32+
* index. The fix for missing aliases folded a {@code remove_index} action into the {@code _aliases}
33+
* request so the delete and the alias-add happen atomically. The {@code remove_index} action was
34+
* built with {@code must_exist(false)}, which OpenSearch's {@code _aliases} parser rejects with
35+
* {@code [remove_index] unknown field [must_exist]} → {@code [aliases] failed to parse field
36+
* [actions]} → HTTP 400. The whole request failed, so the alias-add never applied and the canonical
37+
* name resolved to nothing — surfacing as "table_search_index missing embedding field" in the AI
38+
* Platform validation.
39+
*
40+
* <p>This exercises {@link SearchClient#swapAliases(Set, String, Set, Set)} directly against the
41+
* live cluster with the exact first-install shape (concrete canonical in {@code indicesToRemove}).
42+
* It fails before the fix on OpenSearch and passes after; on Elasticsearch (which tolerates
43+
* {@code must_exist}) it locks down the same correct atomic-swap behavior.
44+
*/
45+
public class AliasSwapConcreteRemovalIT {
46+
47+
private static final String CANONICAL = "om_it_alias_swap_concrete_search_index";
48+
private static final String STAGED = CANONICAL + "_rebuild_it";
49+
private static final String SHORT_ALIAS = "om_it_alias_swap_concrete";
50+
51+
@BeforeAll
52+
static void setup() {
53+
SdkClients.adminClient();
54+
}
55+
56+
@AfterEach
57+
void cleanup() {
58+
SearchClient client = searchClient();
59+
// Delete the staged index first so the canonical alias it carries is removed with it, leaving
60+
// only a concrete canonical (if the swap never ran) for the second delete to clean up.
61+
for (String index : List.of(STAGED, CANONICAL)) {
62+
deleteIfExists(client, index);
63+
}
64+
}
65+
66+
private static void deleteIfExists(SearchClient client, String index) {
67+
if (client.indexExists(index)) {
68+
client.deleteIndex(index);
69+
}
70+
}
71+
72+
@Test
73+
void atomicSwapRemovesConcreteCanonicalAndAttachesAlias() {
74+
SearchClient client = searchClient();
75+
76+
deleteIfExists(client, STAGED);
77+
deleteIfExists(client, CANONICAL);
78+
client.createIndex(CANONICAL, "{}");
79+
client.createIndex(STAGED, "{}");
80+
81+
assertTrue(client.indexExists(CANONICAL), "Concrete canonical index should exist before swap");
82+
assertTrue(
83+
client.getIndicesByAlias(CANONICAL).isEmpty(),
84+
"Canonical name should be a concrete index, not an alias, before swap");
85+
86+
boolean swapped =
87+
client.swapAliases(Set.of(), STAGED, Set.of(CANONICAL, SHORT_ALIAS), Set.of(CANONICAL));
88+
89+
assertTrue(
90+
swapped,
91+
"Atomic swap with a concrete index in indicesToRemove must succeed; before the fix "
92+
+ "OpenSearch rejected the remove_index must_exist field and returned false");
93+
assertEquals(
94+
Set.of(STAGED),
95+
client.getIndicesByAlias(CANONICAL),
96+
"Canonical alias must resolve to the staged index after the swap");
97+
assertTrue(
98+
client.getIndicesByAlias(SHORT_ALIAS).contains(STAGED),
99+
"Short alias must resolve to the staged index after the swap");
100+
}
101+
102+
private static SearchClient searchClient() {
103+
return Entity.getSearchRepository().getSearchClient();
104+
}
105+
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,12 +358,14 @@ public boolean swapAliases(
358358
}
359359
// Then delete any concrete index sharing the alias name, atomically, so the alias
360360
// add below cannot race a separate delete and orphan the canonical name.
361+
// must_exist is intentionally omitted to stay byte-for-byte aligned with the
362+
// OpenSearch path (which rejects it); it is unnecessary because
363+
// resolveCanonicalRemoval only forwards indices already confirmed to exist.
361364
for (String indexToRemove : finalIndicesToRemove) {
362365
updateBuilder.actions(
363366
actionBuilder ->
364367
actionBuilder.removeIndex(
365-
removeIndexBuilder ->
366-
removeIndexBuilder.index(indexToRemove).mustExist(false)));
368+
removeIndexBuilder -> removeIndexBuilder.index(indexToRemove)));
367369
}
368370
// Finally, add aliases to the new index
369371
for (String alias : finalAliases) {

openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchIndexManager.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,12 +439,15 @@ public boolean swapAliases(
439439
}
440440
// Then delete any concrete index sharing the alias name, atomically, so the alias
441441
// add below cannot race a separate delete and orphan the canonical name.
442+
// Do NOT set must_exist: OpenSearch's _aliases parser rejects it on remove_index
443+
// ("unknown field [must_exist]") and fails the whole request. It is unnecessary
444+
// here anyway — resolveCanonicalRemoval only forwards indices it has already
445+
// confirmed exist via indexExists().
442446
for (String indexToRemove : finalIndicesToRemove) {
443447
updateBuilder.actions(
444448
actionBuilder ->
445449
actionBuilder.removeIndex(
446-
removeIndexBuilder ->
447-
removeIndexBuilder.index(indexToRemove).mustExist(false)));
450+
removeIndexBuilder -> removeIndexBuilder.index(indexToRemove)));
448451
}
449452
// Finally, add aliases to the new index
450453
for (String alias : finalAliases) {

0 commit comments

Comments
 (0)